Add precedence test. fixup docs

README.md

@@ -2,10 +2,6 @@
 [![build](https://github.com/SystemRDL/PeakRDL-regblock/workflows/build/badge.svg)](https://github.com/SystemRDL/PeakRDL-regblock/actions?query=workflow%3Abuild+branch%3Amain)
 [![PyPI - Python Version](https://img.shields.io/pypi/pyversions/peakrdl-regblock.svg)](https://pypi.org/project/peakrdl-regblock)
 
-# IMPORTANT
-
-This project has no official releases yet and is still under active development!
-
 # PeakRDL-regblock
 Generate SystemVerilog RTL that implements a register block from compiled SystemRDL input.
 

docs/dev_notes/Validation Needed

@@ -142,10 +142,29 @@ X Warn/error on any signal with cpuif_reset set, that is not in the top-level
     addrmap. At the very least, warn that it will be ignored
 
 
-! "bridge" addrmap not supported
+X "bridge" addrmap not supported
     export shall refuse to process an addrmap marked as a "bridge"
     Only need to check top-level. Compiler will enforce that child nodes arent bridges
 
+X regwidth/accesswidth is sane
+    X accesswidth == regwidth
+        Enforce this for now. Dont feel like supporting fancy modes yet
+        X regwidth < accesswidth
+            This is illegal and is enforced by the compiler.
+        X regwidth > accesswidth
+            Need to extend address decode strobes to have multiple bits
+            this is where looking at endianness matters to determine field placement
+            Dont feel like supporting this yet
+    X constant regwidth?
+        For now, probably limit to only allow the same regwidth everywhere?
+
+
+X Do not allow unaligned addresses
+    All offsets & strides shall be a multiple of the regwidth used
+
+    X each reg needs to be aligned to its width
+    X each regfile/addrmap/stride shall be aligned to the largest regwidth it encloses
+
 ! async data signals
     Only supporting async signals if they are exclusively used in resets.
     Anything else declared as "async" shall emit a warning that it is ignored
@@ -154,27 +173,6 @@ X Warn/error on any signal with cpuif_reset set, that is not in the top-level
 ! Error if a property references a non-signal component, or property reference from
     outside the export hierarchy
 
-! regwidth/accesswidth is sane
-    ! accesswidth == regwidth
-        Enforce this for now. Dont feel like supporting fancy modes yet
-        X regwidth < accesswidth
-            This is illegal and is enforced by the compiler.
-        ! regwidth > accesswidth
-            Need to extend address decode strobes to have multiple bits
-            this is where looking at endinaness matters to determine field placement
-            Dont feel like supporting this yet
-    ! constant regwidth?
-        For now, probably limit to only allow the same regwidth everywhere?
-
-
-! Do not allow unaligned addresses
-    All offsets & strides shall be a multiple of the regwidth used
-
-    - each reg needs to be aligned to its width
-    - each regfile/addrmap/stride shall be aligned to the largest regwidth it encloses
-        --> Should i promote this check to the compiler? At least as a warnable condition
-            Currently i think I only do the more stringent case of block alignment.
-
 ! Add warning for sticky race condition
     stickybit and other similar situations generally should use hw precedence.
     Emit a warning as appropriate

docs/hwif.rst

@@ -48,4 +48,4 @@ For example, a simple design such as:
     hwif_in.my_reg[1].my_field.we
 
 For brevity in this documentation, hwif features will be described using shorthand
-notation that omits the hierarchcal path: ``hwif_out..<feature>``
+notation that omits the hierarchical path: ``hwif_out..<feature>``

docs/index.rst

@@ -2,7 +2,7 @@ Introduction
 ============
 
 PeakRDL-regblock is a free and open-source control & status register (CSR) compiler.
-This code generator that will translate your SystemRDL register descripton into
+This code generator that will translate your SystemRDL register description into
 a synthesizable SystemVerilog RTL module that can be easily instantiated into
 your hardware design.
 
@@ -16,16 +16,11 @@ your hardware design.
 Installing
 ----------
 
-.. important::
-
-   This project has no official releases yet and is still under active development!
-
 Install from `PyPi`_ using pip
 
 .. code-block:: bash
 
-   # NOT RELEASED YET
+   python3 -m pip install peakrdl-regblock
-   # python3 -m pip install peakrdl-regblock
 
 .. _PyPi: https://pypi.org/project/peakrdl-regblock
 

docs/limitations.rst

@@ -29,3 +29,15 @@ No partial writes
 
 Some protocols describe byte-level write strobes. These are not supported yet.
 All write transfers must access the entire register width.
+
+
+Register width, Access width and CPUIF bus width
+------------------------------------------------
+To keep the initial architecture simpler, currently ``regwidth``, ``accesswidth``
+and the resulting CPU bus width has some limitations:
+
+* All registers shall have ``regwidth`` == ``accesswidth``
+* ``regwidth`` shall be the same across all registers within the block being exported.
+* The CPU interface's bus width is statically determined by the ``regwidth`` used.
+
+I have plans to remove these restrictions and allow for more flexibility in the future.

docs/props/field.rst

@@ -533,7 +533,7 @@ paritycheck
 
 precedence
 ^^^^^^^^^^
-|EX|
+|OK|
 
 reset
 ^^^^^

docs/props/signal.rst

@@ -7,19 +7,19 @@ Signal Properties
 
 activehigh/activelow
 --------------------
-|EX|
+|OK|
 
 sync/async
 ----------
-|EX|
+|OK|
 
 Only supported for signals used as resets to infer edge-sensitive reset.
 Ignored in all other contexts.
 
 cpuif_reset
 -----------
-|EX|
+|OK|
 
 field_reset
 -----------
-|EX|
+|OK|

peakrdl/regblock/__about__.py

@@ -1 +1 @@
-__version__ = "0.1.0-a1"
+__version__ = "0.1.0"

peakrdl/regblock/scan_design.py

@@ -1,11 +1,11 @@
-from typing import TYPE_CHECKING, Set
+from typing import TYPE_CHECKING, Set, List
 from collections import OrderedDict
 
 from systemrdl.walker import RDLListener, RDLWalker
-from systemrdl.node import AddrmapNode, SignalNode
+from systemrdl.node import SignalNode, AddressableNode
 
 if TYPE_CHECKING:
-    from systemrdl.node import Node, RegNode, MemNode, FieldNode
+    from systemrdl.node import Node, RegNode, FieldNode
     from .exporter import RegblockExporter
 
 
@@ -21,6 +21,9 @@ class DesignScanner(RDLListener):
         self.cpuif_data_width = 0
         self.msg = exp.top_node.env.msg
 
+        # Keep track of max regwidth encountered in a given block
+        self.max_regwidth_stack = [] # type: List[int]
+
         # Collections of signals that were actually referenced by the design
         self.in_hier_signal_paths = set() # type: Set[str]
         self.out_of_hier_signals = OrderedDict() # type: OrderedDict[str, SignalNode]
@@ -49,6 +52,14 @@ class DesignScanner(RDLListener):
         # collect out-of-hier field_reset, if any
         self._get_out_of_hier_field_reset()
 
+        # Ensure addrmap is not a bridge. This concept does not make sense for
+        # terminal components.
+        if self.exp.top_node.get_property('bridge'):
+            self.msg.error(
+                "Regblock generator does not support exporting bridge address maps",
+                self.exp.top_node.inst.property_src_ref.get('bridge', self.exp.top_node.inst.inst_src_ref)
+            )
+
         RDLWalker().walk(self.exp.top_node, self)
         if self.msg.had_error:
             self.msg.fatal(
@@ -57,11 +68,51 @@ class DesignScanner(RDLListener):
             raise ValueError
 
     def enter_Reg(self, node: 'RegNode') -> None:
+        regwidth = node.get_property('regwidth')
+
+        self.max_regwidth_stack[-1] = max(self.max_regwidth_stack[-1], regwidth)
+
         # The CPUIF's bus width is sized according to the largest register in the design
-        self.cpuif_data_width = max(self.cpuif_data_width, node.get_property('regwidth'))
+        # TODO: make this user-overridable once more flexible regwidth/accesswidths are supported
+        self.cpuif_data_width = max(self.cpuif_data_width, regwidth)
+
+        # TODO: remove this limitation eventually
+        if regwidth != self.cpuif_data_width:
+            self.msg.error(
+                "register blocks with non-uniform regwidths are not supported yet",
+                node.inst.property_src_ref.get('regwidth', node.inst.inst_src_ref)
+            )
+
+        # TODO: remove this limitation eventually
+        if regwidth != node.get_property('accesswidth'):
+            self.msg.error(
+                "Registers that have an accesswidth different from the register width are not supported yet",
+                node.inst.property_src_ref.get('accesswidth', node.inst.inst_src_ref)
+            )
+
+    def enter_AddressableComponent(self, node: AddressableNode) -> None:
+        self.max_regwidth_stack.append(0)
+
+    def exit_AddressableComponent(self, node: AddressableNode) -> None:
+        max_block_regwidth = self.max_regwidth_stack.pop()
+        if self.max_regwidth_stack:
+            self.max_regwidth_stack[-1] = max(self.max_regwidth_stack[-1], max_block_regwidth)
+
+        alignment = int(max_block_regwidth / 8)
+        if (node.raw_address_offset % alignment) != 0:
+            self.msg.error(
+                f"Unaligned registers are not supported. Address offset of instance '{node.inst_name}' must be a multiple of {alignment}",
+                node.inst.inst_src_ref
+            )
+
+        if node.is_array and (node.array_stride % alignment) != 0:
+            self.msg.error(
+                f"Unaligned registers are not supported. Address stride of instance array '{node.inst_name}' must be a multiple of {alignment}",
+                node.inst.inst_src_ref
+            )
 
     def enter_Component(self, node: 'Node') -> None:
-        if not isinstance(node, AddrmapNode) and node.external:
+        if node.external and (node != self.exp.top_node):
             self.msg.error(
                 "Exporter does not support external components",
                 node.inst.inst_src_ref
@@ -93,9 +144,3 @@ class DesignScanner(RDLListener):
                     self.out_of_hier_signals[path] = value
                 else:
                     self.in_hier_signal_paths.add(path)
-
-    def enter_Mem(self, node: 'MemNode') -> None:
-        self.msg.error(
-            "Cannot export a register block that contains a memory",
-            node.inst.inst_src_ref
-        )

test/README.md

@@ -6,10 +6,16 @@
 Testcases require an installation of the Questa simulator, and for `vlog` & `vsim`
 commands to be visible via the PATH environment variable.
 
-*Questa - Intel FPGA Starter Edition* can be downloaded for free from
+*Questa - Intel FPGA Starter Edition* can be downloaded for free from Intel:
-https://fpgasoftware.intel.com/ and is sufficient to run unit tests. You will need
+* Go to https://www.intel.com/content/www/us/en/collections/products/fpga/software/downloads.html?edition=pro&s=Newest
-to generate a free license file to unlock the software: https://licensing.intel.com/psg/s/sales-signup-evaluationlicenses
+* Select latest version of *Intel Quartus Prime Pro*
-
+* Go to the *Individual Files* tab.
+* Download Questa files. (Don't forget part 2!)
+* Install
+* Go to https://licensing.intel.com/psg/s/sales-signup-evaluationlicenses
+* Generate a free *Starter Edition* license file for Questa
+    * Easiest to use a *fixed* license using your NIC ID (MAC address of your network card via `ifconfig`)
+* Download the license file and point the `LM_LICENSE_FILE` environment variable to the folder which contains it.
 
 
 ## Vivado (optional)

test/test_precedence/regblock.rdl

@@ -0,0 +1,26 @@
+addrmap top {
+    reg {
+        field {
+            sw=rw;
+            hw=w; we;
+            precedence=sw;
+        } f_sw = 0;
+        field {
+            sw=rw;
+            hw=w; we;
+            precedence=hw;
+        } f_hw = 0;
+    } r1 @ 0x0;
+
+    reg {
+        default counter;
+        default sw=r;
+        default hw=na;
+
+        field {} f_sw_count[3:0] = 0;
+        field {} f_hw_count[7:4] = 0;
+    } r1_events @ 0x4;
+
+    r1_events.f_sw_count->incr = r1.f_sw;
+    r1_events.f_hw_count->incr = r1.f_hw;
+};

test/test_precedence/tb_template.sv

@@ -0,0 +1,26 @@
+{% extends "lib/tb_base.sv" %}
+
+{% block seq %}
+    {% sv_line_anchor %}
+    ##1;
+    cb.rst <= '0;
+    ##1;
+
+    // Always write both fields from hardware
+    cb.hwif_in.r1.f_sw.next <= '0;
+    cb.hwif_in.r1.f_sw.we <= '1;
+    cb.hwif_in.r1.f_hw.next <= '0;
+    cb.hwif_in.r1.f_hw.we <= '1;
+    @cb;
+    @cb;
+
+    cpuif.assert_read('h0, 'b00);
+    cpuif.assert_read('h4, 'h00);
+
+    cpuif.write('h0, 'b11);
+    cpuif.write('h0, 'b11);
+    cpuif.write('h0, 'b11);
+    cpuif.assert_read('h0, 'h00);
+    cpuif.assert_read('h4, 'h03);
+
+{% endblock %}

test/test_precedence/testcase.py

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