From 378e9242807806b2dc698729847c871f44dd66b3 Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 20 Sep 2019 15:35:55 +0000 Subject: [PATCH] hdl.ast: rename `nbits` to `width`. Also, replace `bits, sign = x.shape()` with more idiomatic `width, signed = x.shape()`. This unifies all properties corresponding to `len(x)` to `x.width`. (Not all values have a `width` property.) Fixes #210. --- nmigen/back/pysim.py | 2 +- nmigen/back/rtlil.py | 24 +++--- nmigen/hdl/ast.py | 134 +++++++++++++++++++--------------- nmigen/hdl/dsl.py | 6 +- nmigen/hdl/mem.py | 8 +- nmigen/hdl/xfrm.py | 4 +- nmigen/test/test_build_res.py | 6 +- nmigen/test/test_hdl_ast.py | 4 +- nmigen/test/test_hdl_mem.py | 8 +- 9 files changed, 106 insertions(+), 90 deletions(-) diff --git a/nmigen/back/pysim.py b/nmigen/back/pysim.py index 57461ee..0f7e8c0 100644 --- a/nmigen/back/pysim.py +++ b/nmigen/back/pysim.py @@ -561,7 +561,7 @@ class Simulator: var_init = signal.decoder(signal.reset).expandtabs().replace(" ", "_") else: var_type = "wire" - var_size = signal.nbits + var_size = signal.width var_init = signal.reset suffix = None diff --git a/nmigen/back/rtlil.py b/nmigen/back/rtlil.py index 460d1d4..c5a0f5d 100644 --- a/nmigen/back/rtlil.py +++ b/nmigen/back/rtlil.py @@ -283,12 +283,12 @@ class _ValueCompilerState: else: wire_name = signal.name - wire_curr = self.rtlil.wire(width=signal.nbits, name=wire_name, + wire_curr = self.rtlil.wire(width=signal.width, name=wire_name, port_id=port_id, port_kind=port_kind, attrs=signal.attrs, src=src(signal.src_loc)) if signal in self.driven and self.driven[signal]: - wire_next = self.rtlil.wire(width=signal.nbits, name=wire_curr + "$next", + wire_next = self.rtlil.wire(width=signal.width, name=wire_curr + "$next", src=src(signal.src_loc)) else: wire_next = None @@ -403,10 +403,10 @@ class _RHSValueCompiler(_ValueCompiler): def on_Const(self, value): if isinstance(value.value, str): - return "{}'{}".format(value.nbits, value.value) + return "{}'{}".format(value.width, value.value) else: - value_twos_compl = value.value & ((1 << value.nbits) - 1) - return "{}'{:0{}b}".format(value.nbits, value_twos_compl, value.nbits) + value_twos_compl = value.value & ((1 << value.width) - 1) + return "{}'{:0{}b}".format(value.width, value_twos_compl, value.width) def on_AnyConst(self, value): if value in self.s.anys: @@ -703,13 +703,13 @@ class _StatementCompiler(xfrm.StatementVisitor): except LegalizeValue as legalize: with self._case.switch(self.rhs_compiler(legalize.value), src=src(legalize.src_loc)) as switch: - bits, sign = legalize.value.shape() - tests = ["{:0{}b}".format(v, bits) for v in legalize.branches] - tests[-1] = "-" * bits + width, signed = legalize.value.shape() + tests = ["{:0{}b}".format(v, width) for v in legalize.branches] + tests[-1] = "-" * width for branch, test in zip(legalize.branches, tests): with self.case(switch, (test,)): self._wrap_assign = False - branch_value = ast.Const(branch, (bits, sign)) + branch_value = ast.Const(branch, (width, signed)) with self.state.expand_to(legalize.value, branch_value): super().on_statement(stmt) self._wrap_assign = True @@ -842,7 +842,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy): if signal not in group_signals: continue if domain is None: - prev_value = ast.Const(signal.reset, signal.nbits) + prev_value = ast.Const(signal.reset, signal.width) else: prev_value = signal case.assign(lhs_compiler(signal), rhs_compiler(prev_value)) @@ -871,7 +871,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy): if signal not in group_signals: continue wire_curr, wire_next = compiler_state.resolve(signal) - sync.update(wire_curr, rhs_compiler(ast.Const(signal.reset, signal.nbits))) + 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. @@ -926,7 +926,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy): if wire in driven: continue wire_curr, _ = compiler_state.wires[wire] - module.connect(wire_curr, rhs_compiler(ast.Const(wire.reset, wire.nbits))) + module.connect(wire_curr, rhs_compiler(ast.Const(wire.reset, wire.width))) # Collect the names we've given to our ports in RTLIL, and correlate these with the signals # represented by these ports. If we are a submodule, this will be necessary to create a cell diff --git a/nmigen/hdl/ast.py b/nmigen/hdl/ast.py index 5e860fe..d9e3d5e 100644 --- a/nmigen/hdl/ast.py +++ b/nmigen/hdl/ast.py @@ -35,14 +35,14 @@ def _enum_shape(enum_type): max_value = max(member.value for member in enum_type) if not isinstance(min_value, int) or not isinstance(max_value, int): raise TypeError("Only enumerations with integer values can be converted to nMigen values") - sign = min_value < 0 or max_value < 0 - bits = max(bits_for(min_value, sign), bits_for(max_value, sign)) - return (bits, sign) + signed = min_value < 0 or max_value < 0 + width = max(bits_for(min_value, signed), bits_for(max_value, signed)) + return (width, signed) def _enum_to_bits(enum_value): - bits, sign = _enum_shape(type(enum_value)) - return format(enum_value.value & ((1 << bits) - 1), "b").rjust(bits, "0") + width, signed = _enum_shape(type(enum_value)) + return format(enum_value.value & ((1 << width) - 1), "b").rjust(width, "0") class Value(metaclass=ABCMeta): @@ -347,24 +347,23 @@ class Const(Value): ---------- value : int shape : int or tuple or None - Either an integer `bits` or a tuple `(bits, signed)` - specifying the number of bits in this `Const` and whether it is - signed (can represent negative values). `shape` defaults - to the minimum width and signedness of `value`. + Either an integer ``width`` or a tuple ``(width, signed)`` specifying the number of bits + in this constant and whether it is signed (can represent negative values). + ``shape`` defaults to the minimum possible width and signedness of ``value``. Attributes ---------- - nbits : int + width : int signed : bool """ src_loc = None @staticmethod def normalize(value, shape): - nbits, signed = shape - mask = (1 << nbits) - 1 + width, signed = shape + mask = (1 << width) - 1 value &= mask - if signed and value >> (nbits - 1): + if signed and value >> (width - 1): value |= ~mask return value @@ -375,14 +374,20 @@ class Const(Value): shape = bits_for(self.value), self.value < 0 if isinstance(shape, int): shape = shape, self.value < 0 - self.nbits, self.signed = shape - if not isinstance(self.nbits, int) or self.nbits < 0: + self.width, self.signed = shape + if not isinstance(self.width, int) or self.width < 0: raise TypeError("Width must be a non-negative integer, not '{!r}'" - .format(self.nbits)) + .format(self.width)) self.value = self.normalize(self.value, shape) + # TODO(nmigen-0.2): move this to nmigen.compat and make it a deprecated extension + @property + @deprecated("instead of `const.nbits`, use `const.width`") + def nbits(self): + return self.width + def shape(self): - return self.nbits, self.signed + return self.width, self.signed def _rhs_signals(self): return ValueSet() @@ -391,7 +396,7 @@ class Const(Value): return self.value def __repr__(self): - return "(const {}'{}d{})".format(self.nbits, "s" if self.signed else "", self.value) + return "(const {}'{}d{})".format(self.width, "s" if self.signed else "", self.value) C = Const # shorthand @@ -402,13 +407,13 @@ class AnyValue(Value, DUID): super().__init__(src_loc_at=src_loc_at) if isinstance(shape, int): shape = shape, False - self.nbits, self.signed = shape - if not isinstance(self.nbits, int) or self.nbits < 0: + self.width, self.signed = shape + if not isinstance(self.width, int) or self.width < 0: raise TypeError("Width must be a non-negative integer, not '{!r}'" - .format(self.nbits)) + .format(self.width)) def shape(self): - return self.nbits, self.signed + return self.width, self.signed def _rhs_signals(self): return ValueSet() @@ -453,41 +458,41 @@ class Operator(Value): def shape(self): op_shapes = list(map(lambda x: x.shape(), self.operands)) if len(op_shapes) == 1: - (a_bits, a_sign), = op_shapes + (a_width, a_signed), = op_shapes if self.op in ("+", "~"): - return a_bits, a_sign + return a_width, a_signed if self.op == "-": - if not a_sign: - return a_bits + 1, True + if not a_signed: + return a_width + 1, True else: - return a_bits, a_sign + return a_width, a_signed if self.op in ("b", "r|", "r&", "r^"): return 1, False elif len(op_shapes) == 2: - (a_bits, a_sign), (b_bits, b_sign) = op_shapes + (a_width, a_signed), (b_width, b_signed) = op_shapes if self.op == "+" or self.op == "-": - bits, sign = self._bitwise_binary_shape(*op_shapes) - return bits + 1, sign + width, signed = self._bitwise_binary_shape(*op_shapes) + return width + 1, signed if self.op == "*": - return a_bits + b_bits, a_sign or b_sign + return a_width + b_width, a_signed or b_signed if self.op == "%": - return a_bits, a_sign + return a_width, a_signed if self.op in ("<", "<=", "==", "!=", ">", ">="): return 1, False if self.op in ("&", "^", "|"): return self._bitwise_binary_shape(*op_shapes) if self.op == "<<": - if b_sign: - extra = 2 ** (b_bits - 1) - 1 + if b_signed: + extra = 2 ** (b_width - 1) - 1 else: - extra = 2 ** (b_bits) - 1 - return a_bits + extra, a_sign + extra = 2 ** (b_width) - 1 + return a_width + extra, a_signed if self.op == ">>": - if b_sign: - extra = 2 ** (b_bits - 1) + if b_signed: + extra = 2 ** (b_width - 1) else: extra = 0 - return a_bits + extra, a_sign + return a_width + extra, a_signed elif len(op_shapes) == 3: if self.op == "m": s_shape, a_shape, b_shape = op_shapes @@ -683,7 +688,7 @@ class Signal(Value, DUID): Parameters ---------- shape : int or tuple or None - Either an integer ``bits`` or a tuple ``(bits, signed)`` specifying the number of bits + Either an integer ``width`` or a tuple ``(width, signed)`` specifying the number of bits in this ``Signal`` and whether it is signed (can represent negative values). ``shape`` defaults to 1-bit and non-signed. name : str @@ -716,7 +721,7 @@ class Signal(Value, DUID): Attributes ---------- - nbits : int + width : int signed : bool name : str reset : int @@ -750,27 +755,27 @@ class Signal(Value, DUID): .format(min, max + 1)) self.signed = min < 0 or max < 0 if min == max: - self.nbits = 0 + self.width = 0 else: - self.nbits = builtins.max(bits_for(min, self.signed), + self.width = builtins.max(bits_for(min, self.signed), bits_for(max, self.signed)) else: if not (min is None and max is None): raise ValueError("Only one of bits/signedness or bounds may be specified") if isinstance(shape, int): - self.nbits, self.signed = shape, False + self.width, self.signed = shape, False else: - self.nbits, self.signed = shape + self.width, self.signed = shape - if not isinstance(self.nbits, int) or self.nbits < 0: - raise TypeError("Width must be a non-negative integer, not '{!r}'".format(self.nbits)) + if not isinstance(self.width, int) or self.width < 0: + raise TypeError("Width must be a non-negative integer, not '{!r}'".format(self.width)) - reset_nbits = bits_for(reset, self.signed) - if reset != 0 and reset_nbits > self.nbits: + reset_width = bits_for(reset, self.signed) + if reset != 0 and reset_width > self.width: warnings.warn("Reset value {!r} requires {} bits to represent, but the signal " "only has {} bits" - .format(reset, reset_nbits, self.nbits), + .format(reset, reset_width, self.width), SyntaxWarning, stacklevel=2 + src_loc_at) self.reset = int(reset) @@ -800,9 +805,9 @@ class Signal(Value, DUID): signed = value_range.start < 0 or (value_range.stop - value_range.step) < 0 else: signed = value_range.start < 0 - nbits = max(bits_for(value_range.start, signed), - bits_for(value_range.stop - value_range.step, signed)) - return cls((nbits, signed), src_loc_at=1 + src_loc_at, **kwargs) + width = max(bits_for(value_range.start, signed), + bits_for(value_range.stop - value_range.step, signed)) + return cls((width, signed), src_loc_at=1 + src_loc_at, **kwargs) @classmethod def enum(cls, enum_type, *, src_loc_at=0, **kwargs): @@ -839,8 +844,19 @@ class Signal(Value, DUID): kw.update(kwargs) return cls(**kw, src_loc_at=1 + src_loc_at) + # TODO(nmigen-0.2): move this to nmigen.compat and make it a deprecated extension + @property + @deprecated("instead of `signal.nbits`, use `signal.width`") + def nbits(self): + return self.width + + @nbits.setter + @deprecated("instead of `signal.nbits = x`, use `signal.width = x`") + def nbits(self, value): + self.width = value + def shape(self): - return self.nbits, self.signed + return self.width, self.signed def _lhs_signals(self): return ValueSet((self,)) @@ -1028,11 +1044,11 @@ class ArrayProxy(Value): return (Value.wrap(elem) for elem in self.elems) def shape(self): - bits, sign = 0, False - for elem_bits, elem_sign in (elem.shape() for elem in self._iter_as_values()): - bits = max(bits, elem_bits + elem_sign) - sign = max(sign, elem_sign) - return bits, sign + width, signed = 0, False + for elem_width, elem_signed in (elem.shape() for elem in self._iter_as_values()): + width = max(width, elem_width + elem_signed) + signed = max(signed, elem_signed) + return width, signed def _lhs_signals(self): signals = union((elem._lhs_signals() for elem in self._iter_as_values()), start=ValueSet()) diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py index 024587f..8a3b561 100644 --- a/nmigen/hdl/dsl.py +++ b/nmigen/hdl/dsl.py @@ -168,8 +168,8 @@ class Module(_ModuleBuilderRoot, Elaboratable): def _check_signed_cond(self, cond): cond = Value.wrap(cond) - bits, sign = cond.shape() - if sign: + width, signed = cond.shape() + if signed: warnings.warn("Signed values in If/Elif conditions usually result from inverting " "Python booleans with ~, which leads to unexpected results: ~True is " "-2, which is truthful. Replace `~flag` with `not flag`. (If this is " @@ -405,7 +405,7 @@ class Module(_ModuleBuilderRoot, Elaboratable): fsm_state_src_locs = data["state_src_locs"] if not fsm_states: return - fsm_signal.nbits = bits_for(len(fsm_encoding) - 1) + fsm_signal.width = bits_for(len(fsm_encoding) - 1) if fsm_reset is None: fsm_signal.reset = fsm_encoding[next(iter(fsm_states))] else: diff --git a/nmigen/hdl/mem.py b/nmigen/hdl/mem.py index 996fca4..181e906 100644 --- a/nmigen/hdl/mem.py +++ b/nmigen/hdl/mem.py @@ -95,8 +95,8 @@ class ReadPort(Elaboratable): def elaborate(self, platform): f = Instance("$memrd", p_MEMID=self.memory, - p_ABITS=self.addr.nbits, - p_WIDTH=self.data.nbits, + p_ABITS=self.addr.width, + p_WIDTH=self.data.width, p_CLK_ENABLE=self.domain != "comb", p_CLK_POLARITY=1, p_TRANSPARENT=self.transparent, @@ -158,8 +158,8 @@ class WritePort(Elaboratable): def elaborate(self, platform): f = Instance("$memwr", p_MEMID=self.memory, - p_ABITS=self.addr.nbits, - p_WIDTH=self.data.nbits, + p_ABITS=self.addr.width, + p_WIDTH=self.data.width, p_CLK_ENABLE=1, p_CLK_POLARITY=1, p_PRIORITY=self.priority, diff --git a/nmigen/hdl/xfrm.py b/nmigen/hdl/xfrm.py index ab12ffd..4e3b9c9 100644 --- a/nmigen/hdl/xfrm.py +++ b/nmigen/hdl/xfrm.py @@ -523,7 +523,7 @@ class DomainLowerer(FragmentTransformer, ValueTransformer, StatementTransformer) domain = fragment.domains[domain_name] if domain.rst is None: continue - stmts = [signal.eq(Const(signal.reset, signal.nbits)) + stmts = [signal.eq(Const(signal.reset, signal.width)) for signal in signals if not signal.reset_less] fragment.add_statements(Switch(domain.rst, {1: stmts})) @@ -732,7 +732,7 @@ class _ControlInserter(FragmentTransformer): class ResetInserter(_ControlInserter): def _insert_control(self, fragment, domain, signals): - stmts = [s.eq(Const(s.reset, s.nbits)) for s in signals if not s.reset_less] + stmts = [s.eq(Const(s.reset, s.width)) for s in signals if not s.reset_less] fragment.add_statements(Switch(self.controls[domain], {1: stmts}, src_loc=self.src_loc)) diff --git a/nmigen/test/test_build_res.py b/nmigen/test/test_build_res.py index 4338770..c212700 100644 --- a/nmigen/test/test_build_res.py +++ b/nmigen/test/test_build_res.py @@ -81,7 +81,7 @@ class ResourceManagerTestCase(FHDLTestCase): self.assertEqual(len(ports), 2) scl, sda = ports self.assertEqual(ports[1].name, "i2c_0__sda__io") - self.assertEqual(ports[1].nbits, 1) + self.assertEqual(ports[1].width, 1) self.assertEqual(list(self.cm.iter_single_ended_pins()), [ (i2c.scl, scl, {}, False), @@ -102,9 +102,9 @@ class ResourceManagerTestCase(FHDLTestCase): self.assertEqual(len(ports), 2) p, n = ports self.assertEqual(p.name, "clk100_0__p") - self.assertEqual(p.nbits, clk100.width) + self.assertEqual(p.width, clk100.width) self.assertEqual(n.name, "clk100_0__n") - self.assertEqual(n.nbits, clk100.width) + self.assertEqual(n.width, clk100.width) self.assertEqual(list(self.cm.iter_differential_pins()), [ (clk100, p, n, {}, False), diff --git a/nmigen/test/test_hdl_ast.py b/nmigen/test/test_hdl_ast.py index e459b3d..9f2fb41 100644 --- a/nmigen/test/test_hdl_ast.py +++ b/nmigen/test/test_hdl_ast.py @@ -381,7 +381,7 @@ class SliceTestCase(FHDLTestCase): class BitSelectTestCase(FHDLTestCase): def setUp(self): self.c = Const(0, 8) - self.s = Signal.range(self.c.nbits) + self.s = Signal.range(self.c.width) def test_shape(self): s1 = self.c.bit_select(self.s, 2) @@ -405,7 +405,7 @@ class BitSelectTestCase(FHDLTestCase): class WordSelectTestCase(FHDLTestCase): def setUp(self): self.c = Const(0, 8) - self.s = Signal.range(self.c.nbits) + self.s = Signal.range(self.c.width) def test_shape(self): s1 = self.c.word_select(self.s, 2) diff --git a/nmigen/test/test_hdl_mem.py b/nmigen/test/test_hdl_mem.py index 13ce7ed..84e1aed 100644 --- a/nmigen/test/test_hdl_mem.py +++ b/nmigen/test/test_hdl_mem.py @@ -123,8 +123,8 @@ class DummyPortTestCase(FHDLTestCase): def test_sizes(self): p1 = DummyPort(width=8, addr_bits=2) - self.assertEqual(p1.addr.nbits, 2) - self.assertEqual(p1.data.nbits, 8) - self.assertEqual(p1.en.nbits, 1) + self.assertEqual(p1.addr.width, 2) + self.assertEqual(p1.data.width, 8) + self.assertEqual(p1.en.width, 1) p2 = DummyPort(width=8, addr_bits=2, granularity=2) - self.assertEqual(p2.en.nbits, 4) + self.assertEqual(p2.en.width, 4) -- 2.30.2