From: whitequark <cz@m-labs.hk>
Date: Fri, 4 Oct 2019 07:49:24 +0000 (+0000)
Subject: hdl.ast: prohibit signed divisors.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=3f2f00e9d8b33ad9dbfb94317d9836bc887d920d;p=nmigen.git

hdl.ast: prohibit signed divisors.

See #238.
---

diff --git a/nmigen/hdl/ast.py b/nmigen/hdl/ast.py
index 6175300..2c997be 100644
--- a/nmigen/hdl/ast.py
+++ b/nmigen/hdl/ast.py
@@ -79,18 +79,34 @@ class Value(metaclass=ABCMeta):
         return Operator("-", [self, other])
     def __rsub__(self, other):
         return Operator("-", [other, self])
+
     def __mul__(self, other):
         return Operator("*", [self, other])
     def __rmul__(self, other):
         return Operator("*", [other, self])
+
+    def __check_divisor(self):
+        width, signed = self.shape()
+        if signed:
+            # Python's division semantics and Verilog's division semantics differ for negative
+            # divisors (Python uses div/mod, Verilog uses quo/rem); for now, avoid the issue
+            # completely by prohibiting such division operations.
+            raise NotImplementedError("Division by a signed value is not supported")
     def __mod__(self, other):
+        other = Value.wrap(other)
+        other.__check_divisor()
         return Operator("%", [self, other])
     def __rmod__(self, other):
+        self.__check_divisor()
         return Operator("%", [other, self])
     def __floordiv__(self, other):
+        other = Value.wrap(other)
+        other.__check_divisor()
         return Operator("//", [self, other])
     def __rfloordiv__(self, other):
+        self.__check_divisor()
         return Operator("//", [other, self])
+
     def __lshift__(self, other):
         return Operator("<<", [self, other])
     def __rlshift__(self, other):
@@ -475,10 +491,8 @@ class Operator(Value):
                 return width + 1, signed
             if self.op == "*":
                 return a_width + b_width, a_signed or b_signed
-            if self.op == "//":
-                # division by -1 can overflow
-                return a_width + b_signed, a_signed or b_signed
-            if self.op == "%":
+            if self.op in ("//", "%"):
+                assert not b_signed
                 return a_width, a_signed
             if self.op in ("<", "<=", "==", "!=", ">", ">="):
                 return 1, False
diff --git a/nmigen/test/test_hdl_ast.py b/nmigen/test/test_hdl_ast.py
index f90d444..0bca37a 100644
--- a/nmigen/test/test_hdl_ast.py
+++ b/nmigen/test/test_hdl_ast.py
@@ -181,17 +181,34 @@ class OperatorTestCase(FHDLTestCase):
         v5 = 10 * Const(0, 4)
         self.assertEqual(v5.shape(), (8, False))
 
+    def test_mod(self):
+        v1 = Const(0, (4, False)) % Const(0, (6, False))
+        self.assertEqual(repr(v1), "(% (const 4'd0) (const 6'd0))")
+        self.assertEqual(v1.shape(), (4, False))
+        v3 = Const(0, (4, True)) % Const(0, (4, False))
+        self.assertEqual(v3.shape(), (4, True))
+        v5 = 10 % Const(0, 4)
+        self.assertEqual(v5.shape(), (4, False))
+
+    def test_mod_wrong(self):
+        with self.assertRaises(NotImplementedError,
+                msg="Division by a signed value is not supported"):
+            Const(0, (4, True)) % Const(0, (6, True))
+
     def test_floordiv(self):
         v1 = Const(0, (4, False)) // Const(0, (6, False))
         self.assertEqual(repr(v1), "(// (const 4'd0) (const 6'd0))")
         self.assertEqual(v1.shape(), (4, False))
-        v2 = Const(0, (4, True)) // Const(0, (6, True))
-        self.assertEqual(v2.shape(), (5, True))
         v3 = Const(0, (4, True)) // Const(0, (4, False))
         self.assertEqual(v3.shape(), (4, True))
         v5 = 10 // Const(0, 4)
         self.assertEqual(v5.shape(), (4, False))
 
+    def test_floordiv_wrong(self):
+        with self.assertRaises(NotImplementedError,
+                msg="Division by a signed value is not supported"):
+            Const(0, (4, True)) // Const(0, (6, True))
+
     def test_and(self):
         v1 = Const(0, (4, False)) & Const(0, (6, False))
         self.assertEqual(repr(v1), "(& (const 4'd0) (const 6'd0))")