From 738d8b7764911af9982365cc59c512e330eb75a4 Mon Sep 17 00:00:00 2001 From: Catherine Date: Tue, 26 Mar 2024 22:28:51 +0000 Subject: [PATCH] hdl: deprecate `{Const,Signal}.{width,signed}` accessors. These accessors used to be necessary (in addition to `.shape()`) while the AST nodes were mutable. However, after commit 2bf1b4da that made AST nodes immutable, there is no technical requirement to keep them around. Additionally: - `len(value)` is shorter than `value.width` and works with any `value` - `value.shape().signed` is longer than `value.signed` but works with any `value` --- amaranth/back/rtlil.py | 15 +++++++------ amaranth/hdl/_ast.py | 50 +++++++++++++++++++++++++----------------- amaranth/hdl/_ir.py | 18 ++++++++------- amaranth/hdl/_mem.py | 2 +- amaranth/hdl/_xfrm.py | 2 +- tests/test_hdl_ast.py | 16 +++++--------- 6 files changed, 56 insertions(+), 47 deletions(-) diff --git a/amaranth/back/rtlil.py b/amaranth/back/rtlil.py index edc1dbf..5c5b7dd 100644 --- a/amaranth/back/rtlil.py +++ b/amaranth/back/rtlil.py @@ -24,7 +24,7 @@ def _signed(value): elif isinstance(value, int): return value < 0 elif isinstance(value, _ast.Const): - return value.signed + return value.shape().signed else: assert False, f"Invalid constant {value!r}" @@ -41,8 +41,8 @@ def _const(value): width = max(32, bits_for(value)) return _const(_ast.Const(value, width)) elif isinstance(value, _ast.Const): - value_twos_compl = value.value & ((1 << value.width) - 1) - return "{}'{:0{}b}".format(value.width, value_twos_compl, value.width) + value_twos_compl = value.value & ((1 << len(value)) - 1) + return "{}'{:0{}b}".format(len(value), value_twos_compl, len(value)) else: assert False, f"Invalid constant {value!r}" @@ -389,9 +389,10 @@ class ModuleEmitter: assert value == port_value self.name_map[signal] = (*self.module.name, name) else: - wire = self.builder.wire(width=signal.width, signed=signal.signed, - name=name, attrs=attrs, - src=_src(signal.src_loc)) + shape = signal.shape() + wire = self.builder.wire(width=shape.width, signed=shape.signed, + name=name, attrs=attrs, + src=_src(signal.src_loc)) self.sigport_wires[name] = (wire, value) self.name_map[signal] = (*self.module.name, wire[1:]) @@ -400,7 +401,7 @@ class ModuleEmitter: for port_id, (name, (value, flow)) in enumerate(self.module.ports.items()): signed = False if name in named_signals: - signed = named_signals[name].signed + signed = named_signals[name].shape().signed wire = self.builder.wire(width=len(value), signed=signed, port_id=port_id, port_kind=flow.value, name=name, attrs=self.value_attrs.get(value, {}), diff --git a/amaranth/hdl/_ast.py b/amaranth/hdl/_ast.py index 551fcc3..cd2b500 100644 --- a/amaranth/hdl/_ast.py +++ b/amaranth/hdl/_ast.py @@ -1542,7 +1542,7 @@ class Const(Value, metaclass=_ConstMeta): width = 0 for part in obj.parts: const = Const.cast(part) - part_value = Const(const.value, unsigned(const.width)).value + part_value = Const(const.value, unsigned(len(const))).value value |= part_value << width width += len(const) return Const(value, width) @@ -1567,34 +1567,40 @@ class Const(Value, metaclass=_ConstMeta): category=SyntaxWarning, stacklevel=3) shape = Shape.cast(shape, src_loc_at=1 + src_loc_at) - self._width = shape.width - self._signed = shape.signed if shape.signed and value >> (shape.width - 1) & 1: value |= -(1 << shape.width) else: value &= (1 << shape.width) - 1 + self._shape = shape self._value = value + def shape(self): + return self._shape + @property def value(self): return self._value + # TODO(amaranth-0.6): remove @property + @deprecated("`const.width` is deprecated and will be removed in Amaranth 0.6; use `len(const)` instead") def width(self): - return self._width + return self.shape().width + # TODO(amaranth-0.6): remove @property + @deprecated("`const.signed` is deprecated and will be removed in Amaranth 0.6; use `const.shape().signed` instead") def signed(self): - return self._signed - - def shape(self): - return Shape(self.width, self.signed) + return self.shape().signed def _rhs_signals(self): return SignalSet() def __repr__(self): - return "(const {}'{}d{})".format(self.width, "s" if self.signed else "", self.value) + if self._shape.signed: + return f"(const {self._shape.width}'sd{self._value})" + else: + return f"(const {self._shape.width}'d{self._value})" C = Const # shorthand @@ -1964,15 +1970,15 @@ class Signal(Value, DUID, metaclass=_SignalMeta): .format(orig_init)) # Avoid false positives for all-zeroes and all-ones if orig_init is not None and not (isinstance(orig_init, int) and orig_init in (0, -1)): - if init.shape().signed and not self.signed: + if init.shape().signed and not self._signed: warnings.warn( message="Initial value {!r} is signed, but the signal shape is {!r}" .format(orig_init, shape), category=SyntaxWarning, stacklevel=2) - elif (init.shape().width > self.width or - init.shape().width == self.width and - self.signed and not init.shape().signed): + elif (init.shape().width > self._width or + init.shape().width == self._width and + self._signed and not init.shape().signed): warnings.warn( message="Initial value {!r} will be truncated to the signal shape {!r}" .format(orig_init, shape), @@ -2030,13 +2036,20 @@ class Signal(Value, DUID, metaclass=_SignalMeta): else: self._decoder = decoder - @property - def width(self): - return self._width + def shape(self): + return Shape(self._width, self._signed) + # TODO(amaranth-0.6): remove @property + @deprecated("`signal.width` is deprecated and will be removed in Amaranth 0.6; use `len(signal)` instead") + def width(self): + return self.shape().width + + # TODO(amaranth-0.6): remove + @property + @deprecated("`signal.signed` is deprecated and will be removed in Amaranth 0.6; use `signal.shape().signed` instead") def signed(self): - return self._signed + return self.shape().signed @property def init(self): @@ -2096,9 +2109,6 @@ class Signal(Value, DUID, metaclass=_SignalMeta): kw["init"] = init return cls(**kw, src_loc_at=1 + src_loc_at) - def shape(self): - return Shape(self.width, self.signed) - def _lhs_signals(self): return SignalSet((self,)) diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py index 9190dfd..04c1d9c 100644 --- a/amaranth/hdl/_ir.py +++ b/amaranth/hdl/_ir.py @@ -625,7 +625,7 @@ class NetlistDriver: def emit_value(self, builder): if self.domain is None: - init = _ast.Const(self.signal.init, self.signal.width) + init = _ast.Const(self.signal.init, len(self.signal)) default, _signed = builder.emit_rhs(self.module_idx, init) else: default = builder.emit_signal(self.signal) @@ -740,11 +740,13 @@ class NetlistEmitter: except KeyError: pass if isinstance(value, _ast.Const): - result = _nir.Value.from_const(value.value, value.width) - signed = value.signed + shape = value.shape() + result = _nir.Value.from_const(value.value, shape.width) + signed = shape.signed elif isinstance(value, _ast.Signal): + shape = value.shape() result = self.emit_signal(value) - signed = value.signed + signed = shape.signed elif isinstance(value, _ast.Operator): if len(value.operands) == 1: operand_a, signed_a = self.emit_rhs(module_idx, value.operands[0]) @@ -1247,12 +1249,12 @@ class NetlistEmitter: else: dir = PortDirection.Input if dir == PortDirection.Input: - top.ports_i[name] = (next_input_bit, signal.width) + top.ports_i[name] = (next_input_bit, len(signal)) value = _nir.Value( _nir.Net.from_cell(0, bit) - for bit in range(next_input_bit, next_input_bit + signal.width) + for bit in range(next_input_bit, next_input_bit + len(signal)) ) - next_input_bit += signal.width + next_input_bit += len(signal) self.connect(signal_value, value, src_loc=signal.src_loc) elif dir == PortDirection.Output: top.ports_o[name] = signal_value @@ -1276,7 +1278,7 @@ class NetlistEmitter: inputs=_nir.Value(cond), src_loc=driver.domain.rst.src_loc) cond, = self.netlist.add_value_cell(1, cell) - init = _nir.Value.from_const(driver.signal.init, driver.signal.width) + init = _nir.Value.from_const(driver.signal.init, len(driver.signal)) driver.assignments.append(_nir.Assignment(cond=cond, start=0, value=init, src_loc=driver.signal.src_loc)) value = driver.emit_value(self) diff --git a/amaranth/hdl/_mem.py b/amaranth/hdl/_mem.py index 20bcd9a..3fd3058 100644 --- a/amaranth/hdl/_mem.py +++ b/amaranth/hdl/_mem.py @@ -44,7 +44,7 @@ class MemoryInstance(Fragment): assert len(self._en) == 1 if domain == "comb": assert isinstance(self._en, Const) - assert self._en.width == 1 + assert self._en.shape() == unsigned(1) assert self._en.value == 1 assert not self._transparent_for diff --git a/amaranth/hdl/_xfrm.py b/amaranth/hdl/_xfrm.py index cd0200e..a193e8a 100644 --- a/amaranth/hdl/_xfrm.py +++ b/amaranth/hdl/_xfrm.py @@ -602,7 +602,7 @@ class _ControlInserter(FragmentTransformer): class ResetInserter(_ControlInserter): def _insert_control(self, fragment, domain, signals): - stmts = [s.eq(Const(s.init, s.width)) for s in signals if not s.reset_less] + stmts = [s.eq(Const(s.init, s.shape())) for s in signals if not s.reset_less] fragment.add_statements(domain, Switch(self.controls[domain], {1: stmts}, src_loc=self.src_loc)) diff --git a/tests/test_hdl_ast.py b/tests/test_hdl_ast.py index 576aa90..4c80e78 100644 --- a/tests/test_hdl_ast.py +++ b/tests/test_hdl_ast.py @@ -897,18 +897,16 @@ class SliceTestCase(FHDLTestCase): def test_const(self): a = Const.cast(Const(0x1234, 16)[4:12]) self.assertEqual(a.value, 0x23) - self.assertEqual(a.width, 8) - self.assertEqual(a.signed, False) + self.assertEqual(a.shape(), unsigned(8)) a = Const.cast(Const(-4, signed(8))[1:6]) self.assertEqual(a.value, 0x1e) - self.assertEqual(a.width, 5) - self.assertEqual(a.signed, False) + self.assertEqual(a.shape(), unsigned(5)) class BitSelectTestCase(FHDLTestCase): def setUp(self): self.c = Const(0, 8) - self.s = Signal(range(self.c.width)) + self.s = Signal(range(len(self.c))) def test_shape(self): s1 = self.c.bit_select(self.s, 2) @@ -946,7 +944,7 @@ class BitSelectTestCase(FHDLTestCase): class WordSelectTestCase(FHDLTestCase): def setUp(self): self.c = Const(0, 8) - self.s = Signal(range(self.c.width)) + self.s = Signal(range(len(self.c))) def test_shape(self): s1 = self.c.word_select(self.s, 2) @@ -1043,12 +1041,10 @@ class CatTestCase(FHDLTestCase): def test_const(self): a = Const.cast(Cat(Const(1, 1), Const(0, 1), Const(3, 2), Const(2, 2))) self.assertEqual(a.value, 0x2d) - self.assertEqual(a.width, 6) - self.assertEqual(a.signed, False) + self.assertEqual(a.shape(), unsigned(6)) a = Const.cast(Cat(Const(-4, 8), Const(-3, 8))) self.assertEqual(a.value, 0xfdfc) - self.assertEqual(a.width, 16) - self.assertEqual(a.signed, False) + self.assertEqual(a.shape(), unsigned(16)) class ArrayTestCase(FHDLTestCase):