Add signed/fixedpoint properties (#140)

* declared intwidth, fracwidth, and is_signed UDPs

* fix boolean type name in UDP definition

* generate hwif fields with fixedpoint indices

* make "counter" and "encode" properties mutualy exclusive with signed/fixedpoint

* add signed/unsigned to hwif

* improved fixedpoint error messages, added validation tests

* added fixedpoint tests

* fixedpoint/signed not allowed for signal components

* added signed/fixedpoint UDP docs

* handle single-bit fixedpoint numbers

* fix too many positional arguments lint

* changed spelling of fixedpoint to fixed-point

* use "logic" in place of "unsigned logic"

* split signed and fixedpoint docs, added examples

* allow enums with is_signed=false

* split signed and fixedpoint implementations

* assorted nits picked

* updated is_signed validation unit test

docs/index.rst

@@ -90,3 +90,5 @@ Links
     udps/read_buffering
     udps/write_buffering
     udps/extended_swacc
+    udps/signed
+    udps/fixedpoint

docs/udps/fixedpoint.rst

@@ -0,0 +1,103 @@
+.. _fixedpoint:
+
+Fixed-Point Fields
+==================
+
+`Fixed-point <https://en.wikipedia.org/wiki/Fixed-point_arithmetic>`_ numbers
+can be used to efficiently represent real numbers using integers. Fixed-point
+numbers consist of some combination of integer bits and fractional bits. The
+number of integer/fractional bits is usually implicitly tracked (not stored)
+for each number, unlike for floating-point numbers.
+
+For this SystemVerilog exporter, these properties only affect the signal type in
+the the ``hwif`` structs. There is no special handling in the internals of
+the regblock.
+
+Properties
+----------
+Fields can be declared as fixed-point numbers using the following two properties:
+
+.. literalinclude:: ../../hdl-src/regblock_udps.rdl
+    :lines: 46-54
+
+The :ref:`is_signed<signed>` property can be used in conjunction with these
+properties to declare signed fixed-point fields.
+
+These UDP definitions, along with others supported by PeakRDL-regblock, can be
+enabled by compiling the following file along with your design:
+:download:`regblock_udps.rdl <../../hdl-src/regblock_udps.rdl>`.
+
+.. describe:: intwidth
+
+    *   The ``intwidth`` property defines the number of integer bits in the
+        fixed-point representation (including the sign bit, if present).
+
+.. describe:: fracwidth
+
+    *   The ``fracwidth`` property defines the number of fractional bits in the
+        fixed-point representation.
+
+Representable Numbers
+^^^^^^^^^^^^^^^^^^^^^
+
+The range of representable real numbers is summarized in the table below.
+
+.. list-table:: Representable Numbers
+    :header-rows: 1
+
+    *   - Signedness
+        - Minimum Value
+        - Maximum Value
+        - Step Size
+
+    *   - Unsigned
+        - :math:`0`
+        - :math:`2^{\mathrm{intwidth}} - 2^{-\mathrm{fracwidth}}`
+        - :math:`2^{-\mathrm{fracwidth}}`
+
+    *   - Signed
+        - :math:`-2^{\mathrm{intwidth}-1}`
+        - :math:`2^{\mathrm{intwidth}-1} - 2^{-\mathrm{fracwidth}}`
+        - :math:`2^{-\mathrm{fracwidth}}`
+
+SystemVerilog Types
+^^^^^^^^^^^^^^^^^^^
+
+When either ``intwidth`` or ``fracwidth`` are defined for a field, that field's
+type in the generated SystemVerilog ``hwif`` struct is
+``logic (signed) [intwidth-1:-fracwidth]``. The bit at index :math:`i` contributes
+a weight of :math:`2^i` to the real number represented.
+
+Other Rules
+^^^^^^^^^^^
+*   Only one of ``intwidth`` or ``fracwidth`` need be defined. The other is
+    inferred from the field bit width.
+*   The bit width of the field shall be equal to ``intwidth`` + ``fracwidth``.
+*   If both ``intwidth`` and ``fracwidth`` are defined for a field,  it is an
+    error if their sum does not equal the bit width of the field.
+*   Either ``fracwidth`` or ``intwidth`` can be a negative integer. Because
+    SystemRDL does not have a signed integer type, the only way to achieve
+    this is to define one of the widths as larger than the bit width of the
+    component so that the other width is inferred as a negative number.
+*   The properties defined above are mutually exclusive with the ``counter``
+    property.
+*   The properties defined above are mutually exclusive with the ``encode``
+    property.
+
+Examples
+--------
+
+A 12-bit signed fixed-point field with 4 integer bits and 8 fractional bits
+can be declared with
+
+.. code-block:: systemrdl
+    :emphasize-lines: 3, 4
+
+    field {
+        sw=rw; hw=r;
+        intwidth = 4;
+        is_signed;
+    } fixedpoint_num[11:0] = 0;
+
+This field can represent values from -8.0 to 7.99609375
+in steps of 0.00390625.

docs/udps/intro.rst

@@ -60,3 +60,26 @@ To enable these UDPs, compile this RDL file prior to the rest of your design:
         - Enables an output strobe that is asserted on sw writes.
 
           See: :ref:`extended_swacc`.
+
+    *   - is_signed
+        - field
+        - boolean
+        - Defines the signedness of a field.
+
+          See: :ref:`signed`.
+
+    *   - intwidth
+        - field
+        - unsigned integer
+        - Defines the number of integer bits in the fixed-point representation
+          of a field.
+
+          See: :ref:`fixedpoint`.
+
+    *   - fracwidth
+        - field
+        - unsigned integer
+        - Defines the number of fractional bits in the fixed-point representation
+          of a field.
+
+          See: :ref:`fixedpoint`.

docs/udps/signed.rst

@@ -0,0 +1,74 @@
+.. _signed:
+
+Signed Fields
+=============
+
+SystemRDL does not natively provide a way to mark fields as signed or unsigned.
+The ``is_signed`` user-defined property fills this need.
+
+For this SystemVerilog exporter, marking a field as signed only affects the
+signal type in the ``hwif`` structs. There is no special handling in the internals
+of the regblock.
+
+Properties
+----------
+A field can be marked as signed using the following user-defined property:
+
+.. literalinclude:: ../../hdl-src/regblock_udps.rdl
+    :lines: 40-44
+
+This UDP definition, along with others supported by PeakRDL-regblock, can be
+enabled by compiling the following file along with your design:
+:download:`regblock_udps.rdl <../../hdl-src/regblock_udps.rdl>`.
+
+.. describe:: is_signed
+
+    *   Assigned value is a boolean.
+    *   If true, the hardware interface field will have the type
+        ``logic signed [width-1:0]``.
+    *   If false or not defined for a field, the hardware interface field will
+        have the type ``logic [width-1:0]``, which is unsigned by definition.
+
+Other Rules
+^^^^^^^^^^^
+
+*   ``is_signed=true`` is mutually exclusive with the ``counter`` property.
+*   ``is_signed=true`` is mutually exclusive with the ``encode`` property.
+
+Examples
+--------
+Below are some examples of fields with different signedness.
+
+Signed Fields
+^^^^^^^^^^^^^
+.. code-block:: systemrdl
+    :emphasize-lines: 3, 8
+
+    field {
+        sw=rw; hw=r;
+        is_signed;
+    } signed_num[63:0] = 0;
+
+    field {
+        sw=r; hw=w;
+        is_signed = true;
+    } another_signed_num[19:0] = 20'hFFFFF; // -1
+
+SystemRDL's own integer type is always unsigned. In order to specify a negative
+reset value, the two's complement value must be used as shown in the second
+example above.
+
+Unsigned Fields
+^^^^^^^^^^^^^^^
+.. code-block:: systemrdl
+    :emphasize-lines: 3, 8
+
+    field {
+        sw=rw; hw=r;
+        // fields are unsigned by default
+    } unsigned_num[63:0] = 0;
+
+    field {
+        sw=r; hw=w;
+        is_signed = false;
+    } another_unsigned_num[19:0] = 0;

hdl-src/regblock_udps.rdl

@@ -36,3 +36,19 @@ property wr_swacc {
     component = field;
     type = boolean;
 };
+
+property is_signed {
+    type = boolean;
+    component = field;
+    default = true;
+};
+
+property intwidth {
+    type = longint unsigned;
+    component = field;
+};
+
+property fracwidth {
+    type = longint unsigned;
+    component = field;
+};

src/peakrdl_regblock/hwif/generators.py

@@ -35,11 +35,11 @@ class HWIFStructGenerator(RDLFlatStructGenerator):
         super().pop_struct()
         self.hwif_report_stack.pop()
 
-    def add_member(self, name: str, width: int = 1) -> None: # type: ignore # pylint: disable=arguments-differ
-        super().add_member(name, width)
+    def add_member(self, name: str, width: int = 1, *, lsb: int = 0, signed: bool = False) -> None: # type: ignore # pylint: disable=arguments-differ
+        super().add_member(name, width, lsb=lsb, signed=signed)
 
-        if width > 1:
-            suffix = f"[{width-1}:0]"
+        if width > 1 or lsb != 0:
+            suffix = f"[{lsb+width-1}:{lsb}]"
         else:
             suffix = ""
 
@@ -145,7 +145,14 @@ class InputStructGenerator_Hier(HWIFStructGenerator):
         # Provide input to field's next value if it is writable by hw, and it
         # was not overridden by the 'next' property
         if node.is_hw_writable and node.get_property('next') is None:
-            self.add_member("next", node.width)
+            # Get the field's LSB index (can be nonzero for fixed-point values)
+            fracwidth = node.get_property("fracwidth")
+            lsb = 0 if fracwidth is None else -fracwidth
+
+            # get the signedness of the field
+            signed = node.get_property("is_signed")
+
+            self.add_member("next", node.width, lsb=lsb, signed=signed)
 
         # Generate implied inputs
         for prop_name in ["we", "wel", "swwe", "swwel", "hwclr", "hwset"]:
@@ -271,7 +278,14 @@ class OutputStructGenerator_Hier(HWIFStructGenerator):
 
         # Expose field's value if it is readable by hw
         if node.is_hw_readable:
-            self.add_member("value", node.width)
+            # Get the node's LSB index (can be nonzero for fixed-point values)
+            fracwidth = node.get_property("fracwidth")
+            lsb = 0 if fracwidth is None else -fracwidth
+
+            # get the signedness of the field
+            signed = node.get_property("is_signed")
+
+            self.add_member("value", node.width, lsb=lsb, signed=signed)
 
         # Generate output bit signals enabled via property
         for prop_name in ["anded", "ored", "xored", "swmod", "swacc", "overflow", "underflow", "rd_swacc", "wr_swacc"]:

src/peakrdl_regblock/struct_generator.py

@@ -88,16 +88,30 @@ class StructGenerator:
         self._struct_stack.append(s)
 
 
-    def add_member(self, name: str, width: int = 1, array_dimensions: Optional[List[int]] = None) -> None:
+    def add_member(
+            self,
+            name: str,
+            width: int = 1,
+            array_dimensions: Optional[List[int]] = None,
+            *,
+            lsb: int = 0,
+            signed: bool = False,
+    ) -> None:
         if array_dimensions:
             suffix = "[" + "][".join((str(n) for n in array_dimensions)) + "]"
         else:
             suffix = ""
 
-        if width == 1:
-            m = f"logic {name}{suffix};"
+        if signed:
+            sign = "signed "
         else:
-            m = f"logic [{width-1}:0] {name}{suffix};"
+            # the default 'logic' type is unsigned per SV LRM 6.11.3
+            sign = ""
+
+        if width == 1 and lsb == 0:
+            m = f"logic {sign}{name}{suffix};"
+        else:
+            m = f"logic {sign}[{lsb+width-1}:{lsb}] {name}{suffix};"
         self.current_struct.children.append(m)
 
 

src/peakrdl_regblock/udps/__init__.py

@@ -1,6 +1,8 @@
 from .rw_buffering import BufferWrites, WBufferTrigger
 from .rw_buffering import BufferReads, RBufferTrigger
 from .extended_swacc import ReadSwacc, WriteSwacc
+from .fixedpoint import IntWidth, FracWidth
+from .signed import IsSigned
 
 ALL_UDPS = [
     BufferWrites,
@@ -9,4 +11,7 @@ ALL_UDPS = [
     RBufferTrigger,
     ReadSwacc,
     WriteSwacc,
+    IntWidth,
+    FracWidth,
+    IsSigned,
 ]

src/peakrdl_regblock/udps/fixedpoint.py

@@ -0,0 +1,73 @@
+from typing import Any
+
+from systemrdl.component import Field
+from systemrdl.node import Node, FieldNode
+from systemrdl.udp import UDPDefinition
+
+
+class _FixedpointWidth(UDPDefinition):
+    valid_components = {Field}
+    valid_type = int
+
+    def validate(self, node: "Node", value: Any) -> None:
+        assert isinstance(node, FieldNode)
+
+        intwidth = node.get_property("intwidth")
+        fracwidth = node.get_property("fracwidth")
+        assert intwidth is not None
+        assert fracwidth is not None
+        prop_ref = node.inst.property_src_ref.get(self.name)
+
+        # incompatible with "counter" fields
+        if node.get_property("counter"):
+            self.msg.error(
+                "Fixed-point representations are not supported for counter fields.",
+                prop_ref
+            )
+
+        # incompatible with "encode" fields
+        if node.get_property("encode") is not None:
+            self.msg.error(
+                "Fixed-point representations are not supported for fields encoded as an enum.",
+                prop_ref
+            )
+
+        # ensure node width = fracwidth + intwidth
+        if intwidth + fracwidth != node.width:
+            self.msg.error(
+                f"Number of integer bits ({intwidth}) plus number of fractional bits ({fracwidth})"
+                f" must be equal to the width of the component ({node.width}).",
+                prop_ref
+            )
+
+
+class IntWidth(_FixedpointWidth):
+    name = "intwidth"
+
+    def get_unassigned_default(self, node: "Node") -> Any:
+        """
+        If 'fracwidth' is defined, 'intwidth' is inferred from the node width.
+        """
+        assert isinstance(node, FieldNode)
+        fracwidth = node.get_property("fracwidth", default=None)
+        if fracwidth is not None:
+            return node.width - fracwidth
+        else:
+            # not a fixed-point number
+            return None
+
+
+class FracWidth(_FixedpointWidth):
+    name = "fracwidth"
+
+    def get_unassigned_default(self, node: "Node") -> Any:
+        """
+        If 'intwidth' is defined, 'fracwidth' is inferred from the node width.
+        """
+        assert isinstance(node, FieldNode)
+        intwidth = node.get_property("intwidth", default=None)
+        if intwidth is not None:
+            return node.width - intwidth
+        else:
+            # not a fixed-point number
+            return None

src/peakrdl_regblock/udps/signed.py

@@ -0,0 +1,33 @@
+from typing import Any
+
+from systemrdl.component import Field
+from systemrdl.node import Node
+from systemrdl.udp import UDPDefinition
+
+
+class IsSigned(UDPDefinition):
+    name = "is_signed"
+    valid_components = {Field}
+    valid_type = bool
+    default_assignment = True
+
+    def validate(self, node: "Node", value: Any) -> None:
+        # "counter" fields can not be signed
+        if value and node.get_property("counter"):
+            self.msg.error(
+                "The property is_signed=true is not supported for counter fields.",
+                node.inst.property_src_ref["is_signed"]
+            )
+
+        # incompatible with "encode" fields
+        if value and node.get_property("encode") is not None:
+            self.msg.error(
+                "The property is_signed=true is not supported for fields encoded as an enum.",
+                node.inst.property_src_ref["is_signed"]
+            )
+
+    def get_unassigned_default(self, node: "Node") -> Any:
+        """
+        Unsigned by default if not specified.
+        """
+        return False

tests/test_fixedpoint/regblock.rdl

@@ -0,0 +1,39 @@
+addrmap top {
+    default accesswidth = 64;
+    default regwidth = 64;
+    reg {
+        field {
+            sw = rw; hw = r;
+            intwidth = 8;
+            fracwidth = 8;
+        } f_Q8_8[16] = 0;
+        field {
+            sw = r; hw = w;
+            intwidth = 32;
+        } f_Q32_n12[20];
+        field {
+            sw = rw; hw = r;
+            fracwidth = 32;
+            is_signed;
+        } f_SQn8_32[24] = 0;
+        field {
+            sw = rw; hw = r;
+            fracwidth = 7;
+            is_signed;
+        } f_SQn6_7 = 0;
+    } r1 @ 0x0;
+
+    reg {
+        field {
+            sw = r; hw = w;
+            is_signed;
+        } f_signed[16];
+        field {
+            sw = rw; hw = r;
+            is_signed = false;
+        } f_unsigned[16] = 0;
+        field {
+            sw = r; hw = w;
+        } f_no_sign[16];
+    } r2 @ 0x8;
+};

tests/test_fixedpoint/tb_template.sv

@@ -0,0 +1,77 @@
+{% extends "lib/tb_base.sv" %}
+
+{% block seq %}
+    {% sv_line_anchor %}
+    ##1;
+    cb.rst <= '0;
+    ##1;
+
+    // set all fields to all 1s
+    cb.hwif_in.r1.f_Q32_n12.next <= '1;
+    cb.hwif_in.r2.f_signed.next <= '1;
+    cb.hwif_in.r2.f_no_sign.next <= '1;
+    cpuif.write('h0, 64'hFFFF_FFFF_FFFF_FFFF);
+    cpuif.write('h8, 64'hFFFF_FFFF_FFFF_FFFF);
+    @cb;
+
+    // Q8.8
+    // verify bit range
+    assert(cb.hwif_out.r1.f_Q8_8.value[7:-8] == '1);
+    // verify bit width
+    assert($size(cb.hwif_out.r1.f_Q8_8.value) == 16);
+    // verfy unsigned
+    assert(cb.hwif_out.r1.f_Q8_8.value > 0);
+
+    // Q32.-12
+    // verify bit range
+    assert(cb.hwif_in.r1.f_Q32_n12.next[31:12] == '1);
+    // verify bit width
+    assert($size(cb.hwif_in.r1.f_Q32_n12.next) == 20);
+    // verify unsigned
+    assert(cb.hwif_in.r1.f_Q32_n12.next > 0);
+
+    // SQ-8.32
+    // verify bit range
+    assert(cb.hwif_out.r1.f_SQn8_32.value[-9:-32] == '1);
+    // verify bit width
+    assert($size(cb.hwif_out.r1.f_SQn8_32.value) == 24);
+    // verify signed
+    assert(cb.hwif_out.r1.f_SQn8_32.value < 0);
+
+    // SQ-6.7
+    // verify bit range
+    assert(cb.hwif_out.r1.f_SQn6_7.value[-7:-7] == '1);
+    // verify bit width
+    assert($size(cb.hwif_out.r1.f_SQn6_7.value) == 1);
+    // verify signed
+    assert(cb.hwif_out.r1.f_SQn6_7.value < 0);
+
+    // 16-bit signed integer
+    // verify bit range
+    assert(cb.hwif_in.r2.f_signed.next[15:0] == '1);
+    // verify bit width
+    assert($size(cb.hwif_in.r2.f_signed.next) == 16);
+    // verify signed
+    assert(cb.hwif_in.r2.f_signed.next < 0);
+
+    // 16-bit unsigned integer
+    // verify bit range
+    assert(cb.hwif_out.r2.f_unsigned.value[15:0] == '1);
+    // verify bit width
+    assert($size(cb.hwif_out.r2.f_unsigned.value) == 16);
+    // verify unsigned
+    assert(cb.hwif_out.r2.f_unsigned.value > 0);
+
+    // 16-bit field (no sign)
+    // verify bit range
+    assert(cb.hwif_in.r2.f_no_sign.next[15:0] == '1);
+    // verify bit width
+    assert($size(cb.hwif_in.r2.f_no_sign.next) == 16);
+    // verify unsigned (logic is unsigned in SV)
+    assert(cb.hwif_in.r2.f_no_sign.next > 0);
+
+    // verify readback
+    cpuif.assert_read('h0, 64'h1FFF_FFFF_FFFF_FFFF);
+    cpuif.assert_read('h8, 64'h0000_FFFF_FFFF_FFFF);
+
+{% endblock %}

tests/test_fixedpoint/testcase.py

@@ -0,0 +1,5 @@
+from ..lib.sim_testcase import SimTestCase
+
+class Test(SimTestCase):
+    def test_dut(self):
+        self.run_test()

tests/test_validation_errors/fixedpoint_counter.rdl

@@ -0,0 +1,9 @@
+addrmap top {
+    reg {
+        field {
+            sw = rw; hw = r;
+            intwidth = 4;
+            counter;
+        } fixedpoint_counter[8] = 0;
+    } r1;
+};

tests/test_validation_errors/fixedpoint_enum.rdl

@@ -0,0 +1,15 @@
+addrmap top {
+    reg {
+        enum test_enum {
+            zero = 2'b00;
+            one = 2'b01;
+            two = 2'b10;
+            three = 2'b11;
+        };
+        field {
+            sw = rw; hw = r;
+            fracwidth = 0;
+            encode = test_enum;
+        } fixedpoint_enum[2] = 0;
+    } r1;
+};

tests/test_validation_errors/fixedpoint_inconsistent_width.rdl

@@ -0,0 +1,9 @@
+addrmap top {
+    reg {
+        field {
+            sw = rw; hw = r;
+            intwidth = 4;
+            fracwidth = 5;
+        } num[8] = 0;
+    } r1;
+};

tests/test_validation_errors/signed_counter.rdl

@@ -0,0 +1,14 @@
+addrmap top {
+    reg {
+        field {
+            sw = rw; hw = r;
+            is_signed = false;
+            counter;
+        } unsigned_counter[8] = 0;
+        field {
+            sw = rw; hw = r;
+            is_signed;
+            counter;
+        } signed_counter[8] = 0;
+    } r1;
+};

tests/test_validation_errors/signed_enum.rdl

@@ -0,0 +1,20 @@
+addrmap top {
+    reg {
+        enum test_enum {
+            zero = 2'b00;
+            one = 2'b01;
+            two = 2'b10;
+            three = 2'b11;
+        };
+        field {
+            sw = rw; hw = r;
+            is_signed = false;
+            encode = test_enum;
+        } unsigned_enum[2] = 0;
+        field {
+            sw = rw; hw = r;
+            is_signed;
+            encode = test_enum;
+        } signed_enum[2] = 0;
+    } r1;
+};

tests/test_validation_errors/testcase.py

@@ -89,3 +89,33 @@ class TestValidationErrors(BaseTestCase):
             "unsynth_reset2.rdl",
             "A field that uses an asynchronous reset cannot use a dynamic reset value. This is not synthesizable.",
         )
+
+    def test_fixedpoint_counter(self) -> None:
+        self.assert_validate_error(
+            "fixedpoint_counter.rdl",
+            "Fixed-point representations are not supported for counter fields.",
+        )
+
+    def test_fixedpoint_enum(self) -> None:
+        self.assert_validate_error(
+            "fixedpoint_enum.rdl",
+            "Fixed-point representations are not supported for fields encoded as an enum.",
+        )
+
+    def test_fixedpoint_inconsistent_width(self) -> None:
+        self.assert_validate_error(
+            "fixedpoint_inconsistent_width.rdl",
+            r"Number of integer bits \(4\) plus number of fractional bits \(5\) must be equal to the width of the component \(8\).",
+        )
+
+    def test_signed_counter(self) -> None:
+        self.assert_validate_error(
+            "signed_counter.rdl",
+            "The property is_signed=true is not supported for counter fields.",
+        )
+
+    def test_signed_enum(self) -> None:
+        self.assert_validate_error(
+            "signed_enum.rdl",
+            "The property is_signed=true is not supported for fields encoded as an enum."
+        )