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.
This commit is contained in:
whitequark 2021-12-11 13:00:46 +00:00
parent 599615ee3a
commit ac13a5b3c9
3 changed files with 31 additions and 10 deletions

View file

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

View file

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

View file

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