hdl.ast: support division and modulo with negative divisor.

Fixes #621.

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

amaranth/back/cxxrtl.py

@@ -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)

amaranth/back/rtlil.py

@@ -428,8 +428,8 @@ class _RHSValueCompiler(_ValueCompiler):
         (2, "+"):    "$add",
         (2, "-"):    "$sub",
         (2, "*"):    "$mul",
-        (2, "//"):   "$div",
+        (2, "//"):   "$divfloor",
-        (2, "%"):    "$mod",
+        (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

amaranth/back/verilog.py

@@ -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))
-
+    script.append("proc -nomux")
-    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 Amaranth does.
-        script.append("proc_init")
-        script.append("proc_arst")
-        script.append("proc_dff")
-        script.append("proc_clean")
     script.append("memory_collect")
 
     if strip_internal_attrs:

amaranth/hdl/ast.py

@@ -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 ("//", "%"):
+            if self.operator == "//":
-                assert not b_signed
+                return Shape(a_width + b_signed, a_signed or b_signed)
-                return Shape(a_width, a_signed)
+            if self.operator == "%":
+                return Shape(b_width, b_signed)
             if self.operator in ("<", "<=", "==", "!=", ">", ">="):
                 return Shape(1, False)
             if self.operator in ("&", "^", "|"):

docs/install.rst

@@ -4,9 +4,11 @@ Installation
 System requirements
 ===================
 
+.. |yosys-version| replace:: 0.10 (or newer)
+
 Amaranth HDL requires Python 3.6; it works on CPython_ 3.6 (or newer), and works faster on PyPy3.6_ 7.2 (or newer).
 
-For most workflows, Amaranth requires Yosys_ 0.9 (or newer). A compatible version of Yosys is distributed via PyPI_ for most popular platforms.
+For most workflows, Amaranth requires Yosys_ |yosys-version|. A compatible version of Yosys is distributed via PyPI_ for most popular platforms.
 
 Simulating Amaranth code requires no additional software. However, a waveform viewer like GTKWave_ is invaluable for debugging.
 
@@ -66,7 +68,7 @@ Installing prerequisites
 
          $ sudo apt-get install yosys
 
-      If Yosys 0.9 (or newer) is not available, `build Yosys from source`_.
+      If Yosys |yosys-version| is not available, `build Yosys from source`_.
 
    .. platform-choice:: arch
       :altname: linux
@@ -85,7 +87,7 @@ Installing prerequisites
 
       On architectures other than |builtin-yosys-architectures|, install Yosys from the package repository of your distribution.
 
-      If Yosys 0.9 (or newer) is not available, `build Yosys from source`_.
+      If Yosys |yosys-version| is not available, `build Yosys from source`_.
 
       .. _build Yosys from source: https://github.com/YosysHQ/yosys/#setup
 

docs/lang.rst

@@ -421,19 +421,17 @@ While arithmetic computations never result in an overflow, :ref:`assigning <lang
 
 The following table lists the arithmetic operations provided by Amaranth:
 
-============ ========================== ======
+============ ==========================
-Operation    Description                Notes
+Operation    Description
-============ ========================== ======
+============ ==========================
 ``a + b``    addition
 ``-a``       negation
 ``a - b``    subtraction
 ``a * b``    multiplication
-``a // b``   floor division             [#opA1]_
+``a // b``   floor division
-``a % b``    modulo                     [#opA1]_
+``a % b``    modulo
 ``abs(a)``   absolute value
-============ ========================== ======
+============ ==========================
-
-.. [#opA1] Divisor must be unsigned; this is an Amaranth limitation that may be lifted in the future.
 
 
 .. _lang-cmpops:

setup.py

@@ -47,7 +47,7 @@ setup(
     ],
     extras_require={
         # this version requirement needs to be synchronized with the one in amaranth.back.verilog!
-        "builtin-yosys": ["amaranth-yosys>=0.9.post3527.*"],
+        "builtin-yosys": ["amaranth-yosys>=0.10.*"],
         "remote-build": ["paramiko~=2.7"],
     },
     packages=find_packages(exclude=("tests", "tests.*")),

tests/test_hdl_ast.py

@@ -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))")

tests/test_sim.py

@@ -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))