hdl.ast: support division and modulo with negative divisor.
authorwhitequark <whitequark@whitequark.org>
Sat, 11 Dec 2021 08:52:14 +0000 (08:52 +0000)
committerLuke Kenneth Casson Leighton <lkcl@lkcl.net>
Fri, 31 Dec 2021 20:10:02 +0000 (20:10 +0000)
Fixes #621.

This commit bumps the Yosys version requirement to >=0.10.

docs/lang.rst
nmigen/back/cxxrtl.py
nmigen/back/rtlil.py
nmigen/back/verilog.py
nmigen/hdl/ast.py
tests/test_hdl_ast.py
tests/test_sim.py

index 0724835918cdb385b9f7e5b28fa1936c53bcd7fc..6e847cdd91f8591c3d99da41bcd5ae8c54ee10e3 100644 (file)
@@ -421,19 +421,17 @@ While arithmetic computations never result in an overflow, :ref:`assigning <lang
 
 The following table lists the arithmetic operations provided by nMigen:
 
-============ ========================== ======
-Operation    Description                Notes
-============ ========================== ======
+============ ==========================
+Operation    Description
+============ ==========================
 ``a + b``    addition
 ``-a``       negation
 ``a - b``    subtraction
 ``a * b``    multiplication
-``a // b``   floor division             [#opA1]_
-``a % b``    modulo                     [#opA1]_
+``a // b``   floor division
+``a % b``    modulo
 ``abs(a)``   absolute value
-============ ========================== ======
-
-.. [#opA1] Divisor must be unsigned; this is an nMigen limitation that may be lifted in the future.
+============ ==========================
 
 
 .. _lang-cmpops:
index 809a136e0f7cc72306483fb722de07d7dab2b6ba..5eb9df1cdee2d2555be2bf2d308f7a0e37bd3fe0 100644 (file)
@@ -18,14 +18,13 @@ def _convert_rtlil_text(rtlil_text, black_boxes, *, src_loc_at=0):
                 raise TypeError("CXXRTL black box source code must be a string, not {!r}"
                                 .format(box_source))
 
-    yosys = find_yosys(lambda ver: ver >= (0, 9, 3468))
+    yosys = find_yosys(lambda ver: ver >= (0, 10))
 
     script = []
     if black_boxes is not None:
         for box_name, box_source in black_boxes.items():
             script.append("read_ilang <<rtlil\n{}\nrtlil".format(box_source))
     script.append("read_ilang <<rtlil\n{}\nrtlil".format(rtlil_text))
-    script.append("delete w:$verilog_initial_trigger")
     script.append("write_cxxrtl")
 
     return yosys.run(["-q", "-"], "\n".join(script), src_loc_at=1 + src_loc_at)
index 306240d53e8e5465ddea9e90dad2702309128636..b3f16e4613df1ccd26cf4e9ce4a6f705b964c3a4 100644 (file)
@@ -428,8 +428,8 @@ class _RHSValueCompiler(_ValueCompiler):
         (2, "+"):    "$add",
         (2, "-"):    "$sub",
         (2, "*"):    "$mul",
-        (2, "//"):   "$div",
-        (2, "%"):    "$mod",
+        (2, "//"):   "$divfloor",
+        (2, "%"):    "$modfloor",
         (2, "**"):   "$pow",
         (2, "<<"):   "$sshl",
         (2, ">>"):   "$sshr",
@@ -825,9 +825,6 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
         lhs_compiler   = _LHSValueCompiler(compiler_state)
         stmt_compiler  = _StatementCompiler(compiler_state, rhs_compiler, lhs_compiler)
 
-        verilog_trigger = None
-        verilog_trigger_sync_emitted = False
-
         # If the fragment is completely empty, add a dummy wire to it, or Yosys will interpret
         # it as a black box by default (when read as Verilog).
         if not fragment.ports and not fragment.statements and not fragment.subfragments:
@@ -942,24 +939,6 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
                     stmt_compiler._wrap_assign = False
                     stmt_compiler(group_stmts)
 
-                    # Verilog `always @*` blocks will not run if `*` does not match anything, i.e.
-                    # if the implicit sensitivity list is empty. We check this while translating,
-                    # by looking for any signals on RHS. If there aren't any, we add some logic
-                    # whose only purpose is to trigger Verilog simulators when it converts
-                    # through RTLIL and to Verilog, by populating the sensitivity list.
-                    #
-                    # Unfortunately, while this workaround allows true (event-driven) Verilog
-                    # simulators to work properly, and is universally ignored by synthesizers,
-                    # Verilator rejects it.
-                    #
-                    # Yosys >=0.9+3468 emits a better workaround on its own, so this code can be
-                    # removed completely once support for Yosys 0.9 is dropped.
-                    if not stmt_compiler._has_rhs:
-                        if verilog_trigger is None:
-                            verilog_trigger = \
-                                module.wire(1, name="$verilog_initial_trigger")
-                        case.assign(verilog_trigger, verilog_trigger)
-
                 # For every signal in the sync domain, assign \sig's initial value (which will
                 # end up as the \init reg attribute) to the reset value.
                 with process.sync("init") as sync:
@@ -969,12 +948,6 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
                         wire_curr, wire_next = compiler_state.resolve(signal)
                         sync.update(wire_curr, rhs_compiler(ast.Const(signal.reset, signal.width)))
 
-                    # The Verilog simulator trigger needs to change at time 0, so if we haven't
-                    # yet done that in some process, do it.
-                    if verilog_trigger and not verilog_trigger_sync_emitted:
-                        sync.update(verilog_trigger, "1'0")
-                        verilog_trigger_sync_emitted = True
-
                 # For every signal in every sync domain, assign \sig to \sig$next. The sensitivity
                 # list, however, differs between domains: for domains with sync reset, it is
                 # `[pos|neg]edge clk`, for sync domains with async reset it is `[pos|neg]edge clk
index 181a765b950c68044e014a8b6a685b04ff383012..b9c32d616eda13568d106ee4dad50328217674b7 100644 (file)
@@ -7,29 +7,12 @@ __all__ = ["YosysError", "convert", "convert_fragment"]
 
 def _convert_rtlil_text(rtlil_text, *, strip_internal_attrs=False, write_verilog_opts=()):
     # this version requirement needs to be synchronized with the one in setup.py!
-    yosys = find_yosys(lambda ver: ver >= (0, 9))
+    yosys = find_yosys(lambda ver: ver >= (0, 10))
     yosys_version = yosys.version()
 
     script = []
     script.append("read_ilang <<rtlil\n{}\nrtlil".format(rtlil_text))
-
-    if yosys_version >= (0, 9, 3468):
-        # Yosys >=0.9+3468 (since commit 128522f1) emits the workaround for the `always @*`
-        # initial scheduling issue on its own.
-        script.append("delete w:$verilog_initial_trigger")
-
-    if yosys_version >= (0, 9, 3527):
-        # Yosys >=0.9+3527 (since commit 656ee70f) supports the `-nomux` option for the `proc`
-        # script pass. Because the individual `proc_*` passes are not a stable interface,
-        # `proc -nomux` is used instead, if available.
-        script.append("proc -nomux")
-    else:
-        # On earlier versions, use individual `proc_*` passes; this is a known range of Yosys
-        # versions and we know it's compatible with what nMigen does.
-        script.append("proc_init")
-        script.append("proc_arst")
-        script.append("proc_dff")
-        script.append("proc_clean")
+    script.append("proc -nomux")
     script.append("memory_collect")
 
     if strip_internal_attrs:
index 0049c0474045d71219495c7b3f396fb427fb8bfa..1bbe9f62eec4e77c573107f0802e386afeb4eef5 100644 (file)
@@ -172,26 +172,13 @@ class Value(metaclass=ABCMeta):
     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.cast(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.cast(other)
-        other.__check_divisor()
         return Operator("//", [self, other])
     def __rfloordiv__(self, other):
-        self.__check_divisor()
         return Operator("//", [other, self])
 
     def __check_shamt(self):
@@ -692,9 +679,10 @@ class Operator(Value):
                 return Shape(width + 1, signed)
             if self.operator == "*":
                 return Shape(a_width + b_width, a_signed or b_signed)
-            if self.operator in ("//", "%"):
-                assert not b_signed
-                return Shape(a_width, a_signed)
+            if self.operator == "//":
+                return Shape(a_width + b_signed, a_signed or b_signed)
+            if self.operator == "%":
+                return Shape(b_width, b_signed)
             if self.operator in ("<", "<=", "==", "!=", ">", ">="):
                 return Shape(1, False)
             if self.operator in ("&", "^", "|"):
index 83f6071a1605dd777c5bccba5bcd1745719b7b96..832e90f67c08e131e6de10d8e7a84bd75e3b65ca 100644 (file)
@@ -407,31 +407,25 @@ class OperatorTestCase(FHDLTestCase):
     def test_mod(self):
         v1 = Const(0, unsigned(4)) % Const(0, unsigned(6))
         self.assertEqual(repr(v1), "(% (const 4'd0) (const 6'd0))")
-        self.assertEqual(v1.shape(), unsigned(4))
+        self.assertEqual(v1.shape(), unsigned(6))
         v3 = Const(0, signed(4)) % Const(0, unsigned(4))
-        self.assertEqual(v3.shape(), signed(4))
+        self.assertEqual(v3.shape(), unsigned(4))
+        v4 = Const(0, signed(4)) % Const(0, signed(6))
+        self.assertEqual(v4.shape(), signed(6))
         v5 = 10 % Const(0, 4)
         self.assertEqual(v5.shape(), unsigned(4))
 
-    def test_mod_wrong(self):
-        with self.assertRaisesRegex(NotImplementedError,
-                r"^Division by a signed value is not supported$"):
-            Const(0, signed(4)) % Const(0, signed(6))
-
     def test_floordiv(self):
         v1 = Const(0, unsigned(4)) // Const(0, unsigned(6))
         self.assertEqual(repr(v1), "(// (const 4'd0) (const 6'd0))")
         self.assertEqual(v1.shape(), unsigned(4))
         v3 = Const(0, signed(4)) // Const(0, unsigned(4))
         self.assertEqual(v3.shape(), signed(4))
+        v4 = Const(0, signed(4)) // Const(0, signed(6))
+        self.assertEqual(v4.shape(), signed(5))
         v5 = 10 // Const(0, 4)
         self.assertEqual(v5.shape(), unsigned(4))
 
-    def test_floordiv_wrong(self):
-        with self.assertRaisesRegex(NotImplementedError,
-                r"^Division by a signed value is not supported$"):
-            Const(0, signed(4)) // Const(0, signed(6))
-
     def test_and(self):
         v1 = Const(0, unsigned(4)) & Const(0, unsigned(6))
         self.assertEqual(repr(v1), "(& (const 4'd0) (const 6'd0))")
index e4bd5c840b9fdbb8e33e1f98c64e7c706aca43c0..a64e90f0d6cc9003ffd8eca5eecfbaccb1036a12 100644 (file)
@@ -116,6 +116,13 @@ class SimulatorUnitTestCase(FHDLTestCase):
         self.assertStatement(stmt, [C(2,  4), C(2,  4)], C(1,   8))
         self.assertStatement(stmt, [C(7,  4), C(2,  4)], C(3,   8))
 
+    def test_floordiv_neg(self):
+        stmt = lambda y, a, b: y.eq(a // b)
+        self.assertStatement(stmt, [C(-5, 4), C( 2, 4)], C(-3, 8))
+        self.assertStatement(stmt, [C(-5, 4), C(-2, 4)], C( 2, 8))
+        self.assertStatement(stmt, [C( 5, 4), C( 2, 4)], C( 2, 8))
+        self.assertStatement(stmt, [C( 5, 4), C(-2, 4)], C(-3, 8))
+
     def test_mod(self):
         stmt = lambda y, a, b: y.eq(a % b)
         self.assertStatement(stmt, [C(2,  4), C(0,  4)], C(0,   8))
@@ -123,6 +130,13 @@ class SimulatorUnitTestCase(FHDLTestCase):
         self.assertStatement(stmt, [C(2,  4), C(2,  4)], C(0,   8))
         self.assertStatement(stmt, [C(7,  4), C(2,  4)], C(1,   8))
 
+    def test_mod_neg(self):
+        stmt = lambda y, a, b: y.eq(a % b)
+        self.assertStatement(stmt, [C(-5, 4), C( 3, 4)], C( 1, 8))
+        self.assertStatement(stmt, [C(-5, 4), C(-3, 4)], C(-2, 8))
+        self.assertStatement(stmt, [C( 5, 4), C( 3, 4)], C( 2, 8))
+        self.assertStatement(stmt, [C( 5, 4), C(-3, 4)], C(-1, 8))
+
     def test_and(self):
         stmt = lambda y, a, b: y.eq(a & b)
         self.assertStatement(stmt, [C(0b1100, 4), C(0b1010, 4)], C(0b1000, 4))