From b452e0e87172c673a9b8beae6497c96a281eb983 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 11 Dec 2021 08:52:14 +0000 Subject: [PATCH] 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 | 3 +-- amaranth/back/rtlil.py | 31 ++----------------------------- amaranth/back/verilog.py | 21 ++------------------- amaranth/hdl/ast.py | 20 ++++---------------- docs/install.rst | 8 +++++--- docs/lang.rst | 14 ++++++-------- setup.py | 2 +- tests/test_hdl_ast.py | 18 ++++++------------ tests/test_sim.py | 14 ++++++++++++++ 9 files changed, 41 insertions(+), 90 deletions(-) diff --git a/amaranth/back/cxxrtl.py b/amaranth/back/cxxrtl.py index 809a136..5eb9df1 100644 --- a/amaranth/back/cxxrtl.py +++ b/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 <>"): "$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 diff --git a/amaranth/back/verilog.py b/amaranth/back/verilog.py index 423e67c..8761cae 100644 --- a/amaranth/back/verilog.py +++ b/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 <= (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("proc -nomux") script.append("memory_collect") if strip_internal_attrs: diff --git a/amaranth/hdl/ast.py b/amaranth/hdl/ast.py index 9c66f3f..7772d71 100644 --- a/amaranth/hdl/ast.py +++ b/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 ("//", "%"): - 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 ("&", "^", "|"): diff --git a/docs/install.rst b/docs/install.rst index f913f44..eeb4a1f 100644 --- a/docs/install.rst +++ b/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 diff --git a/docs/lang.rst b/docs/lang.rst index 65c0c82..b0f9d15 100644 --- a/docs/lang.rst +++ b/docs/lang.rst @@ -421,19 +421,17 @@ While arithmetic computations never result in an overflow, :ref:`assigning =0.9.post3527.*"], + "builtin-yosys": ["amaranth-yosys>=0.10.*"], "remote-build": ["paramiko~=2.7"], }, packages=find_packages(exclude=("tests", "tests.*")), diff --git a/tests/test_hdl_ast.py b/tests/test_hdl_ast.py index 707beac..d116e78 100644 --- a/tests/test_hdl_ast.py +++ b/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))") diff --git a/tests/test_sim.py b/tests/test_sim.py index 98d63c0..1f66fc6 100644 --- a/tests/test_sim.py +++ b/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))