sim._pyrtl: reject very large values.

A check that rejects very large wires already exists in back.rtlil
because they cause performance and correctness issues with Verilog
tooling. Similar performance issues exist with the Python simulator.

This commit also adjusts back.rtlil to use the OverflowError
exception, same as in sim._pyrtl.

Fixes #588.

diff --git a/nmigen/back/rtlil.py b/nmigen/back/rtlil.py
index 873a338e8d96389ec755e57596604d70004040dc..0802b367e6d73dd3f8124528ef8f0b4c48ec2ae6 100644 (file)
--- a/nmigen/back/rtlil.py
+++ b/nmigen/back/rtlil.py
@@ -9,10 +9,6 @@ from ..hdl import ast, ir, mem, xfrm
 __all__ = ["convert", "convert_fragment"]
 
 
-class ImplementationLimit(Exception):
-    pass
-
-
 _escape_map = str.maketrans({
     "\"": "\\\"",
     "\\": "\\\\",
@@ -143,9 +139,9 @@ class _ModuleBuilder(_AttrBuilder, _BufferedBuilder, _Namer):
         # bits. In practice, wires larger than 2**16 bits, although accepted, cause performance
         # problems without an immediately visible cause, so conservatively limit wire size.
         if width > 2 ** 16:
-            raise ImplementationLimit("Wire created at {} is {} bits wide, which is unlikely to "
-                                      "synthesize correctly"
-                                      .format(src or "unknown location", width))
+            raise OverflowError("Wire created at {} is {} bits wide, which is unlikely to "
+                                "synthesize correctly"
+                                .format(src or "unknown location", width))
 
         self._attributes(attrs, src=src, indent=1)
         name = self._make_name(name, local=False)

diff --git a/nmigen/sim/_pyrtl.py b/nmigen/sim/_pyrtl.py
index 13d515f25cb4a6a297726d29901268b8a6a94a59..126ae51c8d543648cbdde3d071531dd7aaa80b8b 100644 (file)
--- a/nmigen/sim/_pyrtl.py
+++ b/nmigen/sim/_pyrtl.py
@@ -70,6 +70,19 @@ class _ValueCompiler(ValueVisitor, _Compiler):
         "zmod": lambda lhs, rhs: 0 if rhs == 0 else lhs % rhs,
     }
 
+    def on_value(self, value):
+        # Very large values are unlikely to compile or simulate in reasonable time.
+        if len(value) > 2 ** 16:
+            if value.src_loc:
+                src = "{}:{}".format(*value.src_loc)
+            else:
+                src = "unknown location"
+            raise OverflowError("Value defined at {} is {} bits wide, which is unlikely to "
+                                "simulate in reasonable time"
+                                .format(src, len(value)))
+
+        return super().on_value(value)
+
     def on_ClockSignal(self, value):
         raise NotImplementedError # :nocov:
 
@@ -332,14 +345,15 @@ class _StatementCompiler(StatementVisitor, _Compiler):
             self.emitter.append("pass")
 
     def on_Assign(self, stmt):
-        gen_rhs = f"({(1 << len(stmt.rhs)) - 1} & {self.rhs(stmt.rhs)})"
+        gen_rhs_value = self.rhs(stmt.rhs) # check for oversized value before generating mask
+        gen_rhs = f"({(1 << len(stmt.rhs)) - 1} & {gen_rhs_value})"
         if stmt.rhs.shape().signed:
             gen_rhs = f"sign({gen_rhs}, {-1 << (len(stmt.rhs) - 1)})"
         return self.lhs(stmt.lhs)(gen_rhs)
 
     def on_Switch(self, stmt):
-        gen_test = self.emitter.def_var("test",
-            f"{(1 << len(stmt.test)) - 1} & {self.rhs(stmt.test)}")
+        gen_test_value = self.rhs(stmt.test) # check for oversized value before generating mask
+        gen_test = self.emitter.def_var("test", f"{(1 << len(stmt.test)) - 1} & {gen_test_value}")
         for index, (patterns, stmts) in enumerate(stmt.cases.items()):
             gen_checks = []
             if not patterns:

diff --git a/tests/test_sim.py b/tests/test_sim.py
index 8d29bfb66620b494461fbe55816ca8c3c39bc9c5..136cdb6e7a97bef5d4a91f436b0e9583ecaec656 100644 (file)
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -840,3 +840,14 @@ class SimulatorRegressionTestCase(FHDLTestCase):
             with open(os.path.devnull, "w") as f:
                 with sim.write_vcd(f):
                     sim.run()
+
+    def test_bug_588(self):
+        dut = Module()
+        a = Signal(32)
+        b = Signal(32)
+        z = Signal(32)
+        dut.d.comb += z.eq(a << b)
+        with self.assertRaisesRegex(OverflowError,
+                r"^Value defined at .+?/test_sim\.py:\d+ is 4294967327 bits wide, "
+                r"which is unlikely to simulate in reasonable time$"):
+            Simulator(dut)