From 7ef4145fc642f3207c3ba70c29e1212d1cc9ff79 Mon Sep 17 00:00:00 2001 From: Prajwal Vandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Tue, 18 Feb 2025 16:09:32 -0800 Subject: [PATCH 1/6] Allow negative Register reset values Pass bitwidth to `infer_val_and_bitwidth` in Register.__init__ to allow negative Register reset values --- pyrtl/wire.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyrtl/wire.py b/pyrtl/wire.py index b5e5e94c..3bf2a9bc 100644 --- a/pyrtl/wire.py +++ b/pyrtl/wire.py @@ -852,7 +852,8 @@ def __init__(self, bitwidth: int, name: str = '', reset_value: int = None, super(Register, self).__init__(bitwidth=bitwidth, name=name, block=block) self.reg_in = None # wire vector setting self.next if reset_value is not None: - reset_value, rst_bitwidth = infer_val_and_bitwidth(reset_value) + # NOTE: pass bitwidth here to allow for negative reset values + reset_value, rst_bitwidth = infer_val_and_bitwidth(reset_value, bitwidth=bitwidth) if rst_bitwidth > bitwidth: raise PyrtlError( 'reset_value "%s" cannot fit in the specified %d bits for this register' From 80f9dc28a38e4acad3e6e0f1d856ad1f884402df Mon Sep 17 00:00:00 2001 From: Prajwal Vandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Sun, 23 Feb 2025 03:10:21 +0000 Subject: [PATCH 2/6] allow negative reset values to Register, add/edit tests for this behavior --- pyrtl/wire.py | 7 +++++-- tests/test_wire.py | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/pyrtl/wire.py b/pyrtl/wire.py index 3bf2a9bc..6da62891 100644 --- a/pyrtl/wire.py +++ b/pyrtl/wire.py @@ -852,8 +852,11 @@ def __init__(self, bitwidth: int, name: str = '', reset_value: int = None, super(Register, self).__init__(bitwidth=bitwidth, name=name, block=block) self.reg_in = None # wire vector setting self.next if reset_value is not None: - # NOTE: pass bitwidth here to allow for negative reset values - reset_value, rst_bitwidth = infer_val_and_bitwidth(reset_value, bitwidth=bitwidth) + # NOTE: passing bitwidth here to allow for negative reset values + reset_value, rst_bitwidth = infer_val_and_bitwidth( + reset_value, + bitwidth=(None if isinstance(reset_value, str) else bitwidth) + ) if rst_bitwidth > bitwidth: raise PyrtlError( 'reset_value "%s" cannot fit in the specified %d bits for this register' diff --git a/tests/test_wire.py b/tests/test_wire.py index 070a4960..285d3173 100644 --- a/tests/test_wire.py +++ b/tests/test_wire.py @@ -263,13 +263,30 @@ def test_reset_value_as_string(self): self.assertEqual(r.reset_value, 1) def test_invalid_reset_value_too_large(self): - with self.assertRaisesRegex(pyrtl.PyrtlError, "cannot fit in the specified"): + with self.assertRaisesRegex( + pyrtl.PyrtlError, + "bitwidth specified \\(4\\) is insufficient to represent constant 16" + ): r = pyrtl.Register(4, reset_value=16) def test_invalid_reset_value_too_large_as_string(self): with self.assertRaisesRegex(pyrtl.PyrtlError, "cannot fit in the specified"): r = pyrtl.Register(4, reset_value="5'd16") + def test_negative_reset_value(self): + r = pyrtl.Register(4, reset_value=-4) + self.assertEqual( + pyrtl.helperfuncs.val_to_signed_integer(r.reset_value, r.bitwidth), + -4 + ) + + def test_negative_reset_value_as_string(self): + r = pyrtl.Register(4, reset_value="-4'd1") + self.assertEqual( + pyrtl.helperfuncs.val_to_signed_integer(r.reset_value, r.bitwidth), + -1 + ) + def test_invalid_reset_value_not_an_integer(self): with self.assertRaises(pyrtl.PyrtlError): r = pyrtl.Register(4, reset_value='hello') From 03c5606d36795ba92ce5d0a046e56536e3cfd533 Mon Sep 17 00:00:00 2001 From: PrajwalVandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Wed, 26 Feb 2025 11:42:59 -0800 Subject: [PATCH 3/6] sign extend negative Verilog string Register reset values, generalize tests --- .gitignore | 3 +++ pyrtl/wire.py | 16 +++++++++++++++- tests/test_wire.py | 18 +++++++++++++----- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 75c349e8..1a33fac9 100644 --- a/.gitignore +++ b/.gitignore @@ -51,3 +51,6 @@ ipynb-examples/.ipynb_checkpoints # VS Code .vscode + +# Python venv +pyvenv.cfg \ No newline at end of file diff --git a/pyrtl/wire.py b/pyrtl/wire.py index 6da62891..f80565f2 100644 --- a/pyrtl/wire.py +++ b/pyrtl/wire.py @@ -852,9 +852,23 @@ def __init__(self, bitwidth: int, name: str = '', reset_value: int = None, super(Register, self).__init__(bitwidth=bitwidth, name=name, block=block) self.reg_in = None # wire vector setting self.next if reset_value is not None: - # NOTE: passing bitwidth here to allow for negative reset values + # detect if reset_value is a negative Verilog constant and sign + # extend it to the correct bitwidth + if isinstance(reset_value, str) and reset_value.startswith('-'): + verilog_bitwidth, constant = reset_value[1:].split("'") + if int(verilog_bitwidth) > bitwidth: + raise PyrtlError( + 'reset_value "%s" cannot fit in the specified %d bits for this register' + % (str(reset_value), bitwidth) + ) + + reset_value = '-' + str(bitwidth) + "'" + constant + reset_value, rst_bitwidth = infer_val_and_bitwidth( reset_value, + # the below allows for Verilog strings with bitwidths less + # than what is specified for the register + # e.g. -3'd6 is a valid reset value for a 4-bit register bitwidth=(None if isinstance(reset_value, str) else bitwidth) ) if rst_bitwidth > bitwidth: diff --git a/tests/test_wire.py b/tests/test_wire.py index 285d3173..c0153def 100644 --- a/tests/test_wire.py +++ b/tests/test_wire.py @@ -263,14 +263,11 @@ def test_reset_value_as_string(self): self.assertEqual(r.reset_value, 1) def test_invalid_reset_value_too_large(self): - with self.assertRaisesRegex( - pyrtl.PyrtlError, - "bitwidth specified \\(4\\) is insufficient to represent constant 16" - ): + with self.assertRaises(pyrtl.PyrtlError): r = pyrtl.Register(4, reset_value=16) def test_invalid_reset_value_too_large_as_string(self): - with self.assertRaisesRegex(pyrtl.PyrtlError, "cannot fit in the specified"): + with self.assertRaises(pyrtl.PyrtlError): r = pyrtl.Register(4, reset_value="5'd16") def test_negative_reset_value(self): @@ -287,6 +284,17 @@ def test_negative_reset_value_as_string(self): -1 ) + def test_invalid_negative_reset_value_as_string(self): + with self.assertRaises(pyrtl.PyrtlError): + r = pyrtl.Register(2, reset_value="-4'd1") + + def test_extending_negative_reset_value_as_string(self): + r = pyrtl.Register(4, reset_value="-3'd3") + self.assertEqual( + pyrtl.helperfuncs.val_to_signed_integer(r.reset_value, r.bitwidth), + -3 + ) + def test_invalid_reset_value_not_an_integer(self): with self.assertRaises(pyrtl.PyrtlError): r = pyrtl.Register(4, reset_value='hello') From dd6808cd3fd3508eb247f5d6f3a3b04b603e71ad Mon Sep 17 00:00:00 2001 From: PrajwalVandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Wed, 26 Feb 2025 13:56:21 -0800 Subject: [PATCH 4/6] move Register reset value sign extension logic to helperfuncs._convert_verilog_str --- pyrtl/helperfuncs.py | 16 ++++++++++------ pyrtl/wire.py | 20 +++----------------- tests/test_wire.py | 1 - 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/pyrtl/helperfuncs.py b/pyrtl/helperfuncs.py index 9731b239..7d4ed39a 100644 --- a/pyrtl/helperfuncs.py +++ b/pyrtl/helperfuncs.py @@ -795,11 +795,20 @@ def _convert_verilog_str(val: str, bitwidth: int = None, if val.startswith('-'): neg = True val = val[1:] + split_string = val.lower().split("'") if len(split_string) != 2: raise PyrtlError('error, string not in verilog style format') try: bitwidth = int(split_string[0]) + if passed_bitwidth is not None: + if bitwidth > passed_bitwidth: + raise PyrtlError( + "bitwidth parameter passed (%d) cannot fit Verilog-style constant with bitwidth %d" + % (passed_bitwidth, bitwidth) + ) + bitwidth = passed_bitwidth + sval = split_string[1] if sval[0] == 's': raise PyrtlError('error, signed integers are not supported in verilog-style constants') @@ -811,17 +820,12 @@ def _convert_verilog_str(val: str, bitwidth: int = None, num = int(sval, base) except (IndexError, ValueError): raise PyrtlError('error, string not in verilog style format') + if neg and num: if (num >> bitwidth - 1): raise PyrtlError('error, insufficient bits for negative number') num = (1 << bitwidth) - num - if passed_bitwidth and passed_bitwidth != bitwidth: - raise PyrtlError('error, bitwidth parameter of constant does not match' - ' the bitwidth infered from the verilog style specification' - ' (if bitwidth=None is used, pyrtl will determine the bitwidth from the' - ' verilog-style constant specification)') - if num >> bitwidth != 0: raise PyrtlError('specified bitwidth %d for verilog constant insufficient to store value %d' % (bitwidth, num)) diff --git a/pyrtl/wire.py b/pyrtl/wire.py index f80565f2..e3b30c9d 100644 --- a/pyrtl/wire.py +++ b/pyrtl/wire.py @@ -852,29 +852,15 @@ def __init__(self, bitwidth: int, name: str = '', reset_value: int = None, super(Register, self).__init__(bitwidth=bitwidth, name=name, block=block) self.reg_in = None # wire vector setting self.next if reset_value is not None: - # detect if reset_value is a negative Verilog constant and sign - # extend it to the correct bitwidth - if isinstance(reset_value, str) and reset_value.startswith('-'): - verilog_bitwidth, constant = reset_value[1:].split("'") - if int(verilog_bitwidth) > bitwidth: - raise PyrtlError( - 'reset_value "%s" cannot fit in the specified %d bits for this register' - % (str(reset_value), bitwidth) - ) - - reset_value = '-' + str(bitwidth) + "'" + constant - reset_value, rst_bitwidth = infer_val_and_bitwidth( reset_value, - # the below allows for Verilog strings with bitwidths less - # than what is specified for the register - # e.g. -3'd6 is a valid reset value for a 4-bit register - bitwidth=(None if isinstance(reset_value, str) else bitwidth) + bitwidth=bitwidth, ) if rst_bitwidth > bitwidth: raise PyrtlError( 'reset_value "%s" cannot fit in the specified %d bits for this register' - % (str(reset_value), bitwidth)) + % (str(reset_value), bitwidth) + ) self.reset_value = reset_value @property diff --git a/tests/test_wire.py b/tests/test_wire.py index c0153def..ea50a1e7 100644 --- a/tests/test_wire.py +++ b/tests/test_wire.py @@ -370,7 +370,6 @@ def test_bad_string(self): self.assert_bad_const("5'b111111'") self.assert_bad_const("'") self.assert_bad_const("'1") - self.assert_bad_const("2'b01", bitwidth=3) self.assert_bad_const("1'") def test_bool(self): From 1fc2e642ff10aa935d135e6923e8c216a1213ca6 Mon Sep 17 00:00:00 2001 From: PrajwalVandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Wed, 26 Feb 2025 17:10:24 -0800 Subject: [PATCH 5/6] slight refactor of _convert_verilog_str and more detailed error msg --- pyrtl/helperfuncs.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pyrtl/helperfuncs.py b/pyrtl/helperfuncs.py index 7d4ed39a..60d1ddcf 100644 --- a/pyrtl/helperfuncs.py +++ b/pyrtl/helperfuncs.py @@ -789,7 +789,6 @@ def _convert_verilog_str(val: str, bitwidth: int = None, raise PyrtlError('error, "signed" option with verilog-style string constants not supported') bases = {'b': 2, 'o': 8, 'd': 10, 'h': 16, 'x': 16} - passed_bitwidth = bitwidth neg = False if val.startswith('-'): @@ -800,18 +799,19 @@ def _convert_verilog_str(val: str, bitwidth: int = None, if len(split_string) != 2: raise PyrtlError('error, string not in verilog style format') try: - bitwidth = int(split_string[0]) - if passed_bitwidth is not None: - if bitwidth > passed_bitwidth: - raise PyrtlError( - "bitwidth parameter passed (%d) cannot fit Verilog-style constant with bitwidth %d" - % (passed_bitwidth, bitwidth) - ) - bitwidth = passed_bitwidth + verilog_bitwidth = int(split_string[0]) + bitwidth = bitwidth or verilog_bitwidth # if bitwidth is None, use verilog_bitwidth + if verilog_bitwidth > bitwidth: + raise PyrtlError( + "bitwidth parameter passed (%d) cannot fit Verilog-style constant with bitwidth %d" + % (bitwidth, verilog_bitwidth) + + " (if bitwidth=None is used, PyRTL will determine the bitwidth from the " + "Verilog-style constant specification)" + ) sval = split_string[1] if sval[0] == 's': - raise PyrtlError('error, signed integers are not supported in verilog-style constants') + raise PyrtlError('error, signed integers are not supported in Verilog-style constants') base = 10 if sval[0] in bases: base = bases[sval[0]] From ed6caa62398da2a4a4610b50aa0977640bac909f Mon Sep 17 00:00:00 2001 From: PrajwalVandana <72575914+PrajwalVandana@users.noreply.github.com> Date: Wed, 26 Feb 2025 17:14:43 -0800 Subject: [PATCH 6/6] fix small code style error --- pyrtl/helperfuncs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyrtl/helperfuncs.py b/pyrtl/helperfuncs.py index 60d1ddcf..b55931c7 100644 --- a/pyrtl/helperfuncs.py +++ b/pyrtl/helperfuncs.py @@ -804,8 +804,8 @@ def _convert_verilog_str(val: str, bitwidth: int = None, if verilog_bitwidth > bitwidth: raise PyrtlError( "bitwidth parameter passed (%d) cannot fit Verilog-style constant with bitwidth %d" - % (bitwidth, verilog_bitwidth) + - " (if bitwidth=None is used, PyRTL will determine the bitwidth from the " + % (bitwidth, verilog_bitwidth) + + " (if bitwidth=None is used, PyRTL will determine the bitwidth from the " "Verilog-style constant specification)" )