First read/write!

doc/Makefile

@@ -0,0 +1,20 @@
+# Minimal makefile for Sphinx documentation
+#
+
+# You can set these variables from the command line, and also
+# from the environment for the first two.
+SPHINXOPTS    ?=
+SPHINXBUILD   ?= sphinx-build
+SOURCEDIR     = .
+BUILDDIR      = _build
+
+# Put it first so that "make" without argument is like "make help".
+help:
+	@$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
+
+.PHONY: help Makefile
+
+# Catch-all target: route all unknown targets to Sphinx using the new
+# "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
+%: Makefile
+	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)

doc/conf.py

@@ -0,0 +1,52 @@
+# Configuration file for the Sphinx documentation builder.
+#
+# This file only contains a selection of the most common options. For a full
+# list see the documentation:
+# https://www.sphinx-doc.org/en/master/usage/configuration.html
+
+# -- Path setup --------------------------------------------------------------
+
+# If extensions (or modules to document with autodoc) are in another directory,
+# add these directories to sys.path here. If the directory is relative to the
+# documentation root, use os.path.abspath to make it absolute, like shown here.
+#
+# import os
+# import sys
+# sys.path.insert(0, os.path.abspath('.'))
+
+
+# -- Project information -----------------------------------------------------
+
+project = 'PeakRDL-regblock'
+copyright = '2021, Alex Mykyta'
+author = 'Alex Mykyta'
+
+
+# -- General configuration ---------------------------------------------------
+
+# Add any Sphinx extension module names here, as strings. They can be
+# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
+# ones.
+extensions = [
+]
+
+# Add any paths that contain templates here, relative to this directory.
+templates_path = ['_templates']
+
+# List of patterns, relative to source directory, that match files and
+# directories to ignore when looking for source files.
+# This pattern also affects html_static_path and html_extra_path.
+exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
+
+
+# -- Options for HTML output -------------------------------------------------
+
+# The theme to use for HTML and HTML Help pages.  See the documentation for
+# a list of builtin themes.
+#
+html_theme = 'alabaster'
+
+# Add any paths that contain custom static files (such as style sheets) here,
+# relative to this directory. They are copied after the builtin static files,
+# so a file named "default.css" will overwrite the builtin "default.css".
+html_static_path = ['_static']

doc/index.rst

@@ -0,0 +1,20 @@
+.. PeakRDL-regblock documentation master file, created by
+   sphinx-quickstart on Tue Nov 16 23:25:58 2021.
+   You can adapt this file completely to your liking, but it should at least
+   contain the root `toctree` directive.
+
+Welcome to PeakRDL-regblock's documentation!
+============================================
+
+.. toctree::
+   :maxdepth: 2
+   :caption: Contents:
+
+
+
+Indices and tables
+==================
+
+* :ref:`genindex`
+* :ref:`modindex`
+* :ref:`search`

doc/logbooks/000-Main-Logbook

@@ -28,6 +28,54 @@ the template would do something like:
 
 Basically, i'd define a ton of helper functions that return the signal identifier.
 
+================================================================================
+Accesswidth vs Regwidth
+================================================================================
+Reading some old versions of the SystemRDL spec (the old "v1 RDL" spec from Cisco)
+it becomes clear that regwidth is actually what defines the bus width!
+
+Some useful points:
+- Section 8.1.3 defines the bus width to be sized according to the superset
+span of msb:lsb fields.
+
+This means that 'accesswidth' is solely for defining the minimum *granularity* of
+an access. For example - APB3 lacks byte strobes, so the bus imposes an accesswidth == regwidth
+APB4 introduces PSTRB, which implies the ability to support an accesswidth of 8
+
+Changes to this tool this new understanding imposes:
+    - derive the CPU bus width based on the largest regwidth
+        this seems like a reasonable & easy thing to implement
+    - CPUIF should make sure to always present an aligned address!
+        if bus width is 32-bits, decoder logic shall recieve an address with bits [1:0] ALWAYS zeroed
+        Codify this in the internal specification!
+    - address decode may produce multiple strobes if registers are packed.
+        Eg: if bus width is 32, and there is a region of 8-bit registers that are tightly packed,
+        an access will strobe four of them at once
+    - readback stage needs to account for narrower registers, and properly
+        pack read values into the response array
+        Remember - the array width is based on the CPUIF width, NOT the reg width
+        Multiple regs can be packed into a cpuif width
+
+So what on earth do I do with accesswidth?
+    - seems to define if sub-accesses are even allowed.
+      I suppose this would be useful to allow/deny such transactions on a per-register basis
+    - for now, enforce that accesswidth == regwidth. This lets me ignore it.
+    - In the future I can ease up on this if I enforce a uniform accesswidth granularity
+      ie: accesswidth can be used, as long as all registers agree to the same value.
+      (unless the regwidth is narrower. thats ok.)
+      eg - OK if:
+        max regwidth = 32
+        all 32-bit registers use 16-bit accesswidth
+        irrelevant to 16 and 8-bit registers
+
+
+Write about this in the SystemRDL errata?
+    Could there be guidance on the CPUIF bus width?
+    For simple protocols like APB, this is meaningful.
+    Maybe not so much in other protocols...
+    Maybe add some words to the "clarifications" section
+
+
 ================================================================================
 Dev Todo list
 ================================================================================
@@ -38,6 +86,32 @@ Dev Todo list
 
 - readback mux
 
+- Start a sphinx docs thing
+    I need to keep better track of everything!
+    Diagrams of each layer in an architecture overview
+    transcribe logbook into dev notes
+
+- Am i doing msb/lsb correctly?
+    bit ordering:
+        fields are always [msb:lsb]
+        but could be [low:high] or [high:low]
+    But how does this affect bus <-> field mapping though??
+        - [low:high] means that bit order gets swapped from the bus
+        - [high:low] means bit order is sliced naturally
+    DONE
+
+
+endianness controls byte order of the CPU bus
+    controls byteswap at the CPUIF layer
+    Internally, use little endian ordering.
+
+    TODO: Add hooks for this in CPUIF layer
+
+Do something about cpuif byte strobes?
+    Remove for now?
+    Demote to APB3?
+
+
 - Other field output assignments
 
 - HWIF layer

doc/logbooks/Validation Needed

@@ -123,8 +123,6 @@ cpuif_resets
     I have zero interest in implementing resynchronizers
 
 ! regwidth/accesswidth is sane
-    ! accesswidth is the same for all registers in the regblock
-        - accesswidth is what implies the cpuif bus width
     ! accesswidth == regwidth
         Enforce this for now. Dont feel like supporting fancy modes yet
         X regwidth < accesswidth
@@ -132,10 +130,18 @@ cpuif_resets
         ! 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?
 
 
 ! Contents of target are all internal. No external regs
 ! Does not contain any mem components
 
 ! Do not allow unaligned addresses
-    All offsets & strides shall be a multiple of the accesswidth used
+    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.

peakrdl/regblock/cpuif/apb3/__init__.py

@@ -1,17 +1,17 @@
 from ..base import CpuifBase
 
-class APB4_Cpuif(CpuifBase):
+class APB3_Cpuif(CpuifBase):
-    template_path = "cpuif/apb4/apb4_tmpl.sv"
+    template_path = "cpuif/apb3/apb3_tmpl.sv"
 
     @property
     def port_declaration(self) -> str:
-        return "apb4_intf.slave s_apb"
+        return "apb3_intf.slave s_apb"
 
     def signal(self, name:str) -> str:
-        return "s_apb." + name
+        return "s_apb." + name.upper()
 
 
-class APB4_Cpuif_flattened(APB4_Cpuif):
+class APB3_Cpuif_flattened(APB3_Cpuif):
     @property
     def port_declaration(self) -> str:
         # TODO: Reference data/addr width from verilog parameter perhaps?
@@ -19,10 +19,8 @@ class APB4_Cpuif_flattened(APB4_Cpuif):
             "input wire " + self.signal("psel"),
             "input wire " + self.signal("penable"),
             "input wire " + self.signal("pwrite"),
-            "input wire " + self.signal("pprot"),
             f"input wire [{self.addr_width-1}:0] " + self.signal("paddr"),
             f"input wire [{self.data_width-1}:0] " + self.signal("pwdata"),
-            f"input wire [{(self.data_width / 8)-1}:0] " + self.signal("pstrb"),
             "output logic " + self.signal("pready"),
             f"output logic [{self.data_width-1}:0] " + self.signal("prdata"),
             "output logic " + self.signal("pslverr"),

peakrdl/regblock/cpuif/apb3/apb3_tmpl.sv

@@ -10,7 +10,6 @@ always_ff {{get_always_ff_event(cpuif_reset)}} begin
         cpuif_req_is_wr <= '0;
         cpuif_addr <= '0;
         cpuif_wr_data <= '0;
-        cpuif_wr_bitstrb <= '0;
     end else begin
         if(~is_active) begin
             if({{cpuif.signal("psel")}}) begin
@@ -19,9 +18,6 @@ always_ff {{get_always_ff_event(cpuif_reset)}} begin
                 cpuif_req_is_wr <= {{cpuif.signal("pwrite")}};
                 cpuif_addr <= {{cpuif.signal("paddr")}}[ADDR_WIDTH-1:0];
                 cpuif_wr_data <= {{cpuif.signal("pwdata")}};
-                for(int i=0; i<DATA_WIDTH/8; i++) begin
-                    cpuif_wr_bitstrb[i*8 +: 8] <= {{"{8{"}}{{cpuif.signal("pstrb")}}[i]{{"}}"}};
-                end
             end
         end else begin
             cpuif_req <= '0;
@@ -31,6 +27,7 @@ always_ff {{get_always_ff_event(cpuif_reset)}} begin
         end
     end
 end
+assign cpuif_wr_bitstrb = '0;
 
 // Response
 assign {{cpuif.signal("pready")}} = cpuif_rd_ack | cpuif_wr_ack;

peakrdl/regblock/dereferencer.py

@@ -68,19 +68,29 @@ class Dereferencer:
             return self.hwif.get_input_identifier(obj)
 
         if isinstance(obj, PropertyReference):
+            if isinstance(obj.node, FieldNode):
+                return self.get_field_propref_value(obj.node, obj.name)
+            elif isinstance(obj.node, RegNode):
+                return self.get_reg_propref_value(obj.node, obj.name)
+            else:
+                raise RuntimeError
 
+        raise RuntimeError("Unhandled reference to: %s" % obj)
+
+
+    def get_field_propref_value(self, field: FieldNode, prop_name: str) -> str:
         # Value reduction properties.
         # Wrap with the appropriate Verilog reduction operator
-            val = self.get_value(obj.node)
+        val = self.get_value(field)
-            if obj.name == "anded":
+        if prop_name == "anded":
             return f"&({val})"
-            elif obj.name == "ored":
+        elif prop_name == "ored":
             return f"|({val})"
-            elif obj.name == "xored":
+        elif prop_name == "xored":
             return f"^({val})"
 
         # references that directly access a property value
-            if obj.name in {
+        if prop_name in {
             'decrvalue',
             'enable',
             'haltenable',
@@ -92,28 +102,39 @@ class Dereferencer:
             'reset',
             'resetsignal',
         }:
-                return self.get_value(obj.node.get_property(obj.name))
+            return self.get_value(field.get_property(prop_name))
-            elif obj.name in {'incr', 'decr'}:
+
-                prop_value = obj.node.get_property(obj.name)
+        # Counter properties
+        if prop_name == 'incr':
+            prop_value = field.get_property(prop_name)
             if prop_value is None:
                 # unset by the user, points to the implied internal signal
-                    return self.field_logic.get_counter_control_identifier(obj)
+                return self.field_logic.get_counter_incr_identifier(field)
             else:
                 return self.get_value(prop_value)
-            elif obj.name == "next":
+        elif prop_name == 'decr':
-                prop_value = obj.node.get_property(obj.name)
+            prop_value = field.get_property(prop_name)
             if prop_value is None:
                 # unset by the user, points to the implied internal signal
-                    return self.field_logic.get_field_next_identifier(obj.node)
+                return self.field_logic.get_counter_decr_identifier(field)
+            else:
+                return self.get_value(prop_value)
+
+        # Field Next
+        if prop_name == "next":
+            prop_value = field.get_property(prop_name)
+            if prop_value is None:
+                # unset by the user, points to the implied internal signal
+                return self.field_logic.get_field_next_identifier(field)
             else:
                 return self.get_value(prop_value)
 
         # References to another component value, or an implied input
-            if obj.name in {'hwclr', 'hwset'}:
+        if prop_name in {'hwclr', 'hwset'}:
-                prop_value = obj.node.get_property(obj.name)
+            prop_value = field.get_property(prop_name)
             if prop_value is True:
                 # Points to inferred hwif input
-                    return self.hwif.get_input_identifier(obj)
+                return self.hwif.get_implied_prop_input_identifier(field, prop_name)
             elif prop_value is False:
                 # This should never happen, as this is checked by the compiler's validator
                 raise RuntimeError
@@ -128,17 +149,17 @@ class Dereferencer:
             "swwe": "swwel",
             "swwel": "swwe",
         }
-            if obj.name in complementary_pairs:
+        if prop_name in complementary_pairs:
-                prop_value = obj.node.get_property(obj.name)
+            prop_value = field.get_property(prop_name)
             if prop_value is True:
                 # Points to inferred hwif input
-                    return self.hwif.get_input_identifier(obj)
+                return self.hwif.get_implied_prop_input_identifier(field, prop_name)
             elif prop_value is False:
                 # Try complementary property
-                    prop_value = obj.node.get_property(complementary_pairs[obj.name])
+                prop_value = field.get_property(complementary_pairs[prop_name])
                 if prop_value is True:
                     # Points to inferred hwif input
-                        return f"!({self.hwif.get_input_identifier(obj)})"
+                    return f"!({self.hwif.get_implied_prop_input_identifier(field, prop_name)})"
                 elif prop_value is False:
                     # This should never happen, as this is checked by the compiler's validator
                     raise RuntimeError
@@ -147,24 +168,29 @@ class Dereferencer:
             else:
                 return self.get_value(prop_value)
 
+        if prop_name == "swacc":
+            return self.field_logic.get_swacc_identifier(field)
+        if prop_name == "swmod":
+            return self.field_logic.get_swmod_identifier(field)
+
+        raise RuntimeError("Unhandled reference to: %s->%s" % (field, prop_name))
+
         """
         TODO:
         Resolves to an internal signal used in the field's logic
             decrsaturate
             decrthreshold
-                halt
             incrsaturate
             incrthreshold
-                intr
             overflow
             saturate
-                swacc
-                swmod
             threshold
         """
 
-        raise RuntimeError("Unhandled reference to: %s", obj)
 
+    def get_reg_propref_value(self, reg: RegNode, prop_name: str) -> str:
+        # TODO: halt, intr
+        raise NotImplementedError
 
 
     def get_access_strobe(self, obj: Union[RegNode, FieldNode]) -> str:

peakrdl/regblock/exporter.py

@@ -7,10 +7,11 @@ from systemrdl.node import AddrmapNode, RootNode
 from .addr_decode import AddressDecode
 from .field_logic import FieldLogic
 from .dereferencer import Dereferencer
-from .readback_mux import ReadbackMux
+from .readback import Readback
 from .signals import InferredSignal, SignalBase
 
-from .cpuif.apb4 import APB4_Cpuif
+from .cpuif import CpuifBase
+from .cpuif.apb3 import APB3_Cpuif
 from .hwif import Hwif
 from .utils import get_always_ff_event
 
@@ -25,11 +26,13 @@ class RegblockExporter:
 
         self.top_node = None # type: AddrmapNode
         self.hwif = None # type: Hwif
+        self.cpuif = None # type: CpuifBase
         self.address_decode = AddressDecode(self)
         self.field_logic = FieldLogic(self)
-        self.readback_mux = ReadbackMux(self)
+        self.readback = Readback(self)
         self.dereferencer = Dereferencer(self)
         self.default_resetsignal = InferredSignal("rst")
+        self.cpuif_reset = self.default_resetsignal
 
 
         if user_template_dir:
@@ -63,7 +66,7 @@ class RegblockExporter:
             self.top_node = node
 
 
-        cpuif_cls = kwargs.pop("cpuif_cls", APB4_Cpuif)
+        cpuif_cls = kwargs.pop("cpuif_cls", APB3_Cpuif)
         hwif_cls = kwargs.pop("hwif_cls", Hwif)
         module_name = kwargs.pop("module_name", self.top_node.inst_name)
         package_name = kwargs.pop("package_name", module_name + "_pkg")
@@ -79,13 +82,13 @@ class RegblockExporter:
         # TODO: Scan design...
 
         # TODO: derive this from somewhere
-        cpuif_reset = self.default_resetsignal
+        self.cpuif_reset = self.default_resetsignal
-        reset_signals = set([cpuif_reset, self.default_resetsignal])
+        reset_signals = set([self.cpuif_reset, self.default_resetsignal])
 
-        cpuif = cpuif_cls(
+        self.cpuif = cpuif_cls(
             self,
-            cpuif_reset=cpuif_reset, # TODO:
+            cpuif_reset=self.cpuif_reset, # TODO:
-            data_width=32, # TODO: derive from the accesswidth used by regs
+            data_width=32, # TODO: derive from the regwidth used by regs
             addr_width=32 # TODO:
         )
 
@@ -100,15 +103,13 @@ class RegblockExporter:
             "data_width": 32, # TODO:
             "addr_width": 32, # TODO:
             "reset_signals": reset_signals,
-            "cpuif_reset": cpuif_reset,
             "user_signals": [], # TODO:
             "interrupts": [], # TODO:
-            "cpuif": cpuif,
+            "cpuif": self.cpuif,
             "hwif": self.hwif,
             "address_decode": self.address_decode,
             "field_logic": self.field_logic,
-            "readback_mux": self.readback_mux,
+            "readback": self.readback,
-            "get_always_ff_event": get_always_ff_event,
         }
 
         # Write out design

peakrdl/regblock/field_logic/__init__.py

@@ -78,7 +78,7 @@ class FieldLogic:
         path = get_indexed_path(self.top_node, node)
         return f"field_combo.{path}.next"
 
-    def get_counter_control_identifier(self, prop_ref: PropertyReference) -> str:
+    def get_counter_incr_identifier(self, field: 'FieldNode') -> str:
         """
         Return the Veriog string that represents the field's inferred incr/decr strobe signal.
         prop_ref will be either an incr or decr property reference, and it is already known that
@@ -87,6 +87,47 @@ class FieldLogic:
         # TODO: Implement this
         raise NotImplementedError
 
+    def get_counter_decr_identifier(self, field: 'FieldNode') -> str:
+        """
+        Return the Veriog string that represents the field's inferred incr/decr strobe signal.
+        prop_ref will be either an incr or decr property reference, and it is already known that
+        the incr/decr properties are not explicitly set by the user and are therefore inferred.
+        """
+        # TODO: Implement this
+        raise NotImplementedError
+
+    def get_swacc_identifier(self, field: 'FieldNode') -> str:
+        """
+        Asserted when field is software accessed (read)
+        """
+        strb = self.exp.dereferencer.get_access_strobe(field)
+        return f"{strb} && !decoded_req_is_wr"
+
+    def get_swmod_identifier(self, field: 'FieldNode') -> str:
+        """
+        Asserted when field is modified by software (written or read with a
+        set or clear side effect).
+        """
+        w_modifiable = field.is_sw_writable
+        r_modifiable = (field.get_property("onread") is not None)
+        strb = self.exp.dereferencer.get_access_strobe(field)
+
+        if w_modifiable and not r_modifiable:
+            # assert swmod only on sw write
+            return f"{strb} && decoded_req_is_wr"
+
+        if w_modifiable and r_modifiable:
+            # assert swmod on all sw actions
+            return strb
+
+        if not w_modifiable and r_modifiable:
+            # assert swmod only on sw read
+            return f"{strb} && !decoded_req_is_wr"
+
+        # Not sw modifiable
+        return "1'b0"
+
+
     #---------------------------------------------------------------------------
     # Field Logic Conditionals
     #---------------------------------------------------------------------------

peakrdl/regblock/field_logic/generators.py

@@ -54,16 +54,20 @@ class FieldLogicGenerator(RDLForLoopGenerator):
     def __init__(self, field_logic: 'FieldLogic'):
         super().__init__()
         self.field_logic = field_logic
-        self.template = self.field_logic.exp.jj_env.get_template(
+        self.exp = field_logic.exp
+        self.field_storage_template = self.field_logic.exp.jj_env.get_template(
             "field_logic/templates/field_storage.sv"
         )
 
 
     def enter_Field(self, node: 'FieldNode') -> None:
-        # If a field doesn't implement storage, it is not relevant here
+        if node.implements_storage:
-        if not node.implements_storage:
+            self.generate_field_storage(node)
-            return
 
+        self.assign_field_outputs(node)
+
+
+    def generate_field_storage(self, node: 'FieldNode') -> None:
         conditionals = self.field_logic.get_conditionals(node)
         extra_combo_signals = OrderedDict()
         for conditional in conditionals:
@@ -74,11 +78,11 @@ class FieldLogicGenerator(RDLForLoopGenerator):
         if sig is not None:
             resetsignal = RDLSignal(sig)
         else:
-            resetsignal = self.field_logic.exp.default_resetsignal
+            resetsignal = self.exp.default_resetsignal
 
         reset_value = node.get_property("reset")
         if reset_value is not None:
-            reset_value_str = self.field_logic.exp.dereferencer.get_value(reset_value)
+            reset_value_str = self.exp.dereferencer.get_value(reset_value)
         else:
             # 5.9.1-g: If no reset value given, the field is not reset, even if it has a resetsignal.
             reset_value_str = None
@@ -87,13 +91,55 @@ class FieldLogicGenerator(RDLForLoopGenerator):
         context = {
             'node': node,
             'reset': reset_value_str,
-            'field_path': get_indexed_path(self.field_logic.top_node, node),
+            'field_path': get_indexed_path(self.exp.top_node, node),
             'extra_combo_signals': extra_combo_signals,
             'conditionals': conditionals,
             'resetsignal': resetsignal,
             'get_always_ff_event': get_always_ff_event,
-            'has_value_output': self.field_logic.exp.hwif.has_value_output,
-            'get_output_identifier': self.field_logic.exp.hwif.get_output_identifier,
-
         }
-        self.add_content(self.template.render(context))
+        self.add_content(self.field_storage_template.render(context))
+
+
+    def assign_field_outputs(self, node: 'FieldNode') -> None:
+        field_path = get_indexed_path(self.exp.top_node, node)
+
+        # Field value output
+        if self.exp.hwif.has_value_output(node):
+            output_identifier = self.exp.hwif.get_output_identifier(node)
+            self.add_content(
+                f"assign {output_identifier} = field_storage.{field_path};"
+            )
+
+        # Inferred logical reduction outputs
+        if node.get_property("anded"):
+            output_identifier = self.exp.hwif.get_implied_prop_output_identifier(node, "anded")
+            value = self.exp.dereferencer.get_field_propref_value(node, "anded")
+            self.add_content(
+                f"assign {output_identifier} = {value};"
+            )
+        if node.get_property("ored"):
+            output_identifier = self.exp.hwif.get_implied_prop_output_identifier(node, "ored")
+            value = self.exp.dereferencer.get_field_propref_value(node, "ored")
+            self.add_content(
+                f"assign {output_identifier} = {value};"
+            )
+        if node.get_property("xored"):
+            output_identifier = self.exp.hwif.get_implied_prop_output_identifier(node, "xored")
+            value = self.exp.dereferencer.get_field_propref_value(node, "xored")
+            self.add_content(
+                f"assign {output_identifier} = {value};"
+            )
+
+        if node.get_property("swmod"):
+            output_identifier = self.exp.hwif.get_implied_prop_output_identifier(node, "swmod")
+            value = self.field_logic.get_swmod_identifier(node)
+            self.add_content(
+                f"assign {output_identifier} = {value};"
+            )
+
+        if node.get_property("swacc"):
+            output_identifier = self.exp.hwif.get_implied_prop_output_identifier(node, "swacc")
+            value = self.field_logic.get_swacc_identifier(node)
+            self.add_content(
+                f"assign {output_identifier} = {value};"
+            )

peakrdl/regblock/field_logic/sw_onread.py

@@ -14,7 +14,8 @@ class _OnRead(NextStateConditional):
 
     def get_predicate(self, field: 'FieldNode') -> str:
         strb = self.exp.dereferencer.get_access_strobe(field)
-        return f"decoded_req && !decoded_req_is_wr && {strb}"
+        return f"{strb} && !decoded_req_is_wr"
+
 
 class ClearOnRead(_OnRead):
     comment = "SW clear on read"

peakrdl/regblock/field_logic/sw_onwrite.py

@@ -8,6 +8,7 @@ if TYPE_CHECKING:
     from systemrdl.node import FieldNode
 
 # TODO: implement sw=w1 "write once" fields
+# TODO: Implement swwe/swwel masking properties
 
 class _OnWrite(NextStateConditional):
     onwritetype = None
@@ -16,7 +17,15 @@ class _OnWrite(NextStateConditional):
 
     def get_predicate(self, field: 'FieldNode') -> str:
         strb = self.exp.dereferencer.get_access_strobe(field)
-        return f"decoded_req && decoded_req_is_wr && {strb}"
+        return f"{strb} && decoded_req_is_wr"
+
+    def _wr_data(self, field: 'FieldNode') -> str:
+        if field.msb < field.lsb:
+            # Field gets bitswapped since it is in [low:high] orientation
+            value = f"{{<<{{decoded_wr_data[{field.high}:{field.low}]}}}}"
+        else:
+            value = f"decoded_wr_data[{field.high}:{field.low}]"
+        return value
 
 class WriteOneSet(_OnWrite):
     comment = "SW write 1 set"
@@ -25,7 +34,7 @@ class WriteOneSet(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} | decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} | {self._wr_data(field)}];",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -36,7 +45,7 @@ class WriteOneClear(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} & ~decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} & ~{self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -47,7 +56,7 @@ class WriteOneToggle(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} ^ decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} ^ {self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -58,7 +67,7 @@ class WriteZeroSet(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} | ~decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} | ~{self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -69,7 +78,7 @@ class WriteZeroClear(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} & decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} & {self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -80,7 +89,7 @@ class WriteZeroToggle(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = field_storage.{field_path} ^ ~decoded_wr_data;",
+            f"field_combo.{field_path}.next = field_storage.{field_path} ^ ~{self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]
 
@@ -113,6 +122,6 @@ class Write(_OnWrite):
     def get_assignments(self, field: 'FieldNode') -> List[str]:
         field_path = self.get_field_path(field)
         return [
-            f"field_combo.{field_path}.next = decoded_wr_data;",
+            f"field_combo.{field_path}.next = {self._wr_data(field)};",
             f"field_combo.{field_path}.load_next = '1;",
         ]

peakrdl/regblock/field_logic/templates/field_storage.sv

@@ -21,6 +21,3 @@ always_ff {{get_always_ff_event(resetsignal)}} begin
         field_storage.{{field_path}} <= field_combo.{{field_path}}.next;
     end
 end
-{% if has_value_output(node) -%}
-    assign {{get_output_identifier(node)}} = field_storage.{{field_path}};
-{%- endif -%}

peakrdl/regblock/forloop_generator.py

@@ -31,9 +31,9 @@ class LoopBody(Body):
         )
 
 
-
 class ForLoopGenerator:
     i_type = "int"
+    loop_body_cls = LoopBody
 
     def __init__(self) -> None:
         self._loop_level = 0
@@ -45,7 +45,7 @@ class ForLoopGenerator:
 
     def push_loop(self, dim: int) -> None:
         i = f"i{self._loop_level}"
-        b = LoopBody(dim, i, self.i_type)
+        b = self.loop_body_cls(dim, i, self.i_type)
         self._stack.append(b)
         self._loop_level += 1
 

peakrdl/regblock/hwif.py

@@ -201,7 +201,6 @@ class Hwif:
         for prop_name in ["anded", "ored", "xored", "swmod", "swacc"]:
             if node.get_property(prop_name):
                 contents.append(f"logic {prop_name};")
-        # TODO: Are there was_written/was_read strobes too?
 
         return contents
 
@@ -247,9 +246,7 @@ class Hwif:
             # TODO: Implement this
             raise NotImplementedError()
         elif isinstance(obj, PropertyReference):
-            assert obj.name in {'hwclr', 'hwset', 'swwe', 'swwel', 'we', 'wel'}
+            return self.get_implied_prop_input_identifier(obj.node, obj.name)
-            path = get_indexed_path(self.top_node, obj.node)
-            return "hwif_in." + path + "." + obj.name
 
         raise RuntimeError("Unhandled reference to: %s", obj)
 
@@ -274,10 +271,11 @@ class Hwif:
             path = get_indexed_path(self.top_node, obj)
             return "hwif_out." + path + ".value"
         elif isinstance(obj, PropertyReference):
-            assert obj.name in {"anded", "ored", "xored", "swmod", "swacc"}
+            # TODO: this might be dead code.
+            # not sure when anything would call this function with a prop ref
+            # when dereferencer's get_value is more useful here
             assert obj.node.get_property(obj.name)
-            path = get_indexed_path(self.top_node, obj.node)
+            return self.get_implied_prop_output_identifier(obj.node, obj.name)
-            return "hwif_out." + path + "." + obj.name
 
         raise RuntimeError("Unhandled reference to: %s", obj)
 

peakrdl/regblock/module_tmpl.sv

@@ -13,7 +13,8 @@ module {{module_name}} (
         {{interrupt.port_declaration}},
         {%- endfor %}
 
-        {{cpuif.port_declaration|indent(8)}},
+        {{cpuif.port_declaration|indent(8)}}
+        {%- if hwif.has_input_struct or hwif.has_output_struct %},{% endif %}
 
         {{hwif.port_declaration|indent(8)}}
     );
@@ -71,27 +72,10 @@ module {{module_name}} (
     {{field_logic.get_storage_struct()|indent}}
 
     {{field_logic.get_implementation()|indent}}
-    // TODO: output port signal assignment (aka output mapping layer)
 
     //--------------------------------------------------------------------------
-    // Readback mux
+    // Readback
     //--------------------------------------------------------------------------
-    logic readback_err;
+    {{readback.get_implementation()|indent}}
-    logic readback_done;
-    logic [DATA_WIDTH-1:0] readback_data;
-
-    {{readback_mux.get_implementation()|indent}}
-
-    always_ff {{get_always_ff_event(cpuif_reset)}} begin
-        if({{cpuif_reset.activehigh_identifier}}) begin
-            cpuif_rd_ack <= '0;
-            cpuif_rd_data <= '0;
-            cpuif_rd_err <= '0;
-        end else begin
-            cpuif_rd_ack <= readback_done;
-            cpuif_rd_data <= readback_data;
-            cpuif_rd_err <= readback_err;
-        end
-    end
 
 endmodule

peakrdl/regblock/readback/__init__.py

@@ -0,0 +1,31 @@
+from typing import TYPE_CHECKING
+
+from .generators import ReadbackAssignmentGenerator
+from ..utils import get_always_ff_event
+
+if TYPE_CHECKING:
+    from ..exporter import RegblockExporter
+    from systemrdl.node import AddrmapNode
+
+class Readback:
+    def __init__(self, exp:'RegblockExporter'):
+        self.exp = exp
+
+    @property
+    def top_node(self) -> 'AddrmapNode':
+        return self.exp.top_node
+
+    def get_implementation(self) -> str:
+        gen = ReadbackAssignmentGenerator(self.exp)
+        array_assignments = gen.get_content(self.top_node)
+
+        context = {
+            "array_assignments" : array_assignments,
+            "array_size" : gen.current_offset,
+            "get_always_ff_event": get_always_ff_event,
+            "cpuif_reset": self.exp.cpuif_reset,
+        }
+        template = self.exp.jj_env.get_template(
+            "readback/templates/readback.sv"
+        )
+        return template.render(context)

peakrdl/regblock/readback/generators.py

@@ -0,0 +1,107 @@
+from typing import TYPE_CHECKING
+
+from ..forloop_generator import RDLForLoopGenerator, LoopBody
+
+if TYPE_CHECKING:
+    from ..exporter import RegblockExporter
+    from systemrdl.node import RegNode
+
+class ReadbackLoopBody(LoopBody):
+    def __init__(self, dim: int, iterator: str, i_type: str) -> None:
+        super().__init__(dim, iterator, i_type)
+        self.n_regs = 0
+
+    def __str__(self) -> str:
+        # replace $i#sz token when stringifying
+        s = super().__str__()
+        token = f"${self.iterator}sz"
+        s = s.replace(token, str(self.n_regs))
+        return s
+
+class ReadbackAssignmentGenerator(RDLForLoopGenerator):
+    i_type = "genvar"
+    loop_body_cls = ReadbackLoopBody
+
+    def __init__(self, exp:'RegblockExporter') -> None:
+        super().__init__()
+        self.exp = exp
+
+        # The readback array collects all possible readback values into a flat
+        # array. The array width is equal to the CPUIF bus width. Each entry in
+        # the array represents an aligned read access.
+        self.current_offset = 0
+        self.start_offset_stack = []
+        self.dim_stack = []
+
+    @property
+    def current_offset_str(self) -> str:
+        """
+        Derive a string that represents the current offset being assigned.
+        This consists of:
+        - The current integer offset
+        - multiplied index of any enclosing loop
+
+        The integer offset from "current_offset" is static and is monotonically
+        incremented as more register assignments are processed.
+
+        The component of the offset from loops is added by multiplying the current
+        loop index by the loop size.
+        Since the loop's size is not known at this time, it is emitted as a
+        placeholder token like: $i0sz, $i1sz, $i2sz, etc
+        These tokens can be replaced once the loop body has been completed and the
+        size of its contents is known.
+        """
+        offset_parts = []
+        for i in range(self._loop_level):
+            offset_parts.append(f"i{i}*$i{i}sz")
+        offset_parts.append(str(self.current_offset))
+        return " + ".join(offset_parts)
+
+    def enter_Reg(self, node: 'RegNode') -> None:
+        # TODO: account for smaller regs that are not aligned to the bus width
+        #   - offset the field bit slice as appropriate
+        #   - do not always increment the current offset
+        if node.has_sw_readable:
+            current_bit = 0
+            rd_strb = f"({self.exp.dereferencer.get_access_strobe(node)} && !decoded_req_is_wr)"
+            # Fields are sorted by ascending low bit
+            for field in node.fields():
+                if field.is_sw_readable:
+                    # insert reserved assignment before if needed
+                    if field.low != current_bit:
+                        self.add_content(f"assign readback_array[{self.current_offset_str}][{field.low-1}:{current_bit}] = '0;")
+
+                    if field.msb < field.lsb:
+                        # Field gets bitswapped since it is in [low:high] orientation
+                        value = f"{{<<{{{self.exp.dereferencer.get_value(field)}}}}}"
+                    else:
+                        value = self.exp.dereferencer.get_value(field)
+
+                    self.add_content(f"assign readback_array[{self.current_offset_str}][{field.high}:{field.low}] = {rd_strb} ? {value} : '0;")
+
+                    current_bit = field.high + 1
+
+            # Insert final reserved assignment if needed
+            bus_width = self.exp.cpuif.data_width
+            if current_bit < bus_width:
+                self.add_content(f"assign readback_array[{self.current_offset_str}][{bus_width-1}:{current_bit}] = '0;")
+
+            self.current_offset += 1
+
+    def push_loop(self, dim: int) -> None:
+        super().push_loop(dim)
+        self.start_offset_stack.append(self.current_offset)
+        self.dim_stack.append(dim)
+
+    def pop_loop(self) -> None:
+        start_offset = self.start_offset_stack.pop()
+        dim = self.dim_stack.pop()
+
+        # Number of registers enclosed in this loop
+        n_regs = self.current_offset - start_offset
+        self.current_loop.n_regs = n_regs
+
+        super().pop_loop()
+
+        # Advance current scope's offset to account for loop's contents
+        self.current_offset += n_regs * dim - 1

peakrdl/regblock/readback/templates/readback.sv

@@ -0,0 +1,45 @@
+{% if array_assignments is not none %}
+logic readback_err;
+logic readback_done;
+logic [DATA_WIDTH-1:0] readback_data;
+logic [DATA_WIDTH-1:0] readback_array[{{array_size}}];
+
+{{array_assignments}}
+
+always_comb begin
+    automatic logic [DATA_WIDTH-1:0] readback_data_var;
+    readback_done = decoded_req & ~decoded_req_is_wr;
+    readback_err = '0;
+
+    readback_data_var = '0;
+    for(int i=0; i<{{array_size}}; i++) begin
+        readback_data_var |= readback_array[i];
+    end
+    readback_data = readback_data_var;
+end
+
+always_ff {{get_always_ff_event(cpuif_reset)}} begin
+    if({{cpuif_reset.activehigh_identifier}}) begin
+        cpuif_rd_ack <= '0;
+        cpuif_rd_data <= '0;
+        cpuif_rd_err <= '0;
+    end else begin
+        cpuif_rd_ack <= readback_done;
+        cpuif_rd_data <= readback_data;
+        cpuif_rd_err <= readback_err;
+    end
+end
+
+
+
+{%- else %}
+always_ff {{get_always_ff_event(cpuif_reset)}} begin
+    if({{cpuif_reset.activehigh_identifier}}) begin
+        cpuif_rd_ack <= '0;
+    end else begin
+        cpuif_rd_ack <= decoded_req & ~decoded_req_is_wr;
+    end
+end
+assign cpuif_rd_data = '0;
+assign cpuif_rd_err = '0;
+{% endif %}

peakrdl/regblock/readback_mux.py

@@ -1,23 +0,0 @@
-import re
-from typing import TYPE_CHECKING, List
-
-from systemrdl.node import AddrmapNode
-
-
-if TYPE_CHECKING:
-    from .exporter import RegblockExporter
-
-class ReadbackMux:
-    def __init__(self, exp:'RegblockExporter'):
-        self.exp = exp
-
-    @property
-    def top_node(self) -> AddrmapNode:
-        return self.exp.top_node
-
-    def get_implementation(self) -> str:
-        # TODO: Count the number of readable registers
-        # TODO: Emit the declaration for the readback array
-        # TODO: Always comb block to assign & mask all elements
-        # TODO: Separate always_comb block to OR reduce down
-        return "//TODO"

test/.gitignore

@@ -0,0 +1,3 @@
+work
+transcript
+*.wlf

test/drivers/apb3_intf_driver.sv

@@ -0,0 +1,95 @@
+interface apb3_intf_driver #(
+        parameter DATA_WIDTH = 32,
+        parameter ADDR_WIDTH = 32
+    )(
+        input wire clk,
+        apb3_intf.master m_apb
+    );
+
+    timeunit 1ps;
+    timeprecision 1ps;
+
+    logic PSEL;
+    logic PENABLE;
+    logic PWRITE;
+    logic [ADDR_WIDTH-1:0] PADDR;
+    logic [DATA_WIDTH-1:0] PWDATA;
+    logic [DATA_WIDTH-1:0] PRDATA;
+    logic PREADY;
+    logic PSLVERR;
+
+    assign m_apb.PSEL = PSEL;
+    assign m_apb.PENABLE = PENABLE;
+    assign m_apb.PWRITE = PWRITE;
+    assign m_apb.PADDR = PADDR;
+    assign m_apb.PWDATA = PWDATA;
+    assign PRDATA = m_apb.PRDATA;
+    assign PREADY = m_apb.PREADY;
+    assign PSLVERR = m_apb.PSLVERR;
+
+    default clocking cb @(posedge clk);
+        default input #1step output #1;
+        output PSEL;
+        output PENABLE;
+        output PWRITE;
+        output PADDR;
+        output PWDATA;
+        input PRDATA;
+        input PREADY;
+        input PSLVERR;
+    endclocking
+
+    task reset();
+        cb.PSEL <= '0;
+        cb.PENABLE <= '0;
+        cb.PWRITE <= '0;
+        cb.PADDR <= '0;
+        cb.PWDATA <= '0;
+    endtask
+
+    task write(logic [ADDR_WIDTH-1:0] addr, logic [DATA_WIDTH-1:0] data);
+        ##0;
+
+        // Initiate transfer
+        cb.PSEL <= '1;
+        cb.PENABLE <= '0;
+        cb.PWRITE <= '1;
+        cb.PADDR <= addr;
+        cb.PWDATA <= data;
+        @(cb);
+
+        // active phase
+        cb.PENABLE <= '1;
+        @(cb);
+
+        // Wait for response
+        while(cb.PREADY !== 1'b1) @(cb);
+        reset();
+    endtask
+
+    task read(logic [ADDR_WIDTH-1:0] addr, output logic [DATA_WIDTH-1:0] data);
+        ##0;
+
+        // Initiate transfer
+        cb.PSEL <= '1;
+        cb.PENABLE <= '0;
+        cb.PWRITE <= '0;
+        cb.PADDR <= addr;
+        cb.PWDATA <= '0;
+        @(cb);
+
+        // active phase
+        cb.PENABLE <= '1;
+        @(cb);
+
+        // Wait for response
+        while(cb.PREADY !== 1'b1) @(cb);
+        data = cb.PRDATA;
+        reset();
+    endtask
+
+    initial begin
+        reset();
+    end
+
+endinterface

test/interfaces/apb3_intf.sv

@@ -0,0 +1,40 @@
+interface apb3_intf #(
+    parameter DATA_WIDTH = 32,
+    parameter ADDR_WIDTH = 32
+);
+    // Command
+    logic PSEL;
+    logic PENABLE;
+    logic PWRITE;
+    logic [ADDR_WIDTH-1:0] PADDR;
+    logic [DATA_WIDTH-1:0] PWDATA;
+
+    // Response
+    logic [DATA_WIDTH-1:0] PRDATA;
+    logic PREADY;
+    logic PSLVERR;
+
+    modport master (
+        output PSEL,
+        output PENABLE,
+        output PWRITE,
+        output PADDR,
+        output PWDATA,
+
+        input PRDATA,
+        input PREADY,
+        input PSLVERR
+    );
+
+    modport slave (
+        input PSEL,
+        input PENABLE,
+        input PWRITE,
+        input PADDR,
+        input PWDATA,
+
+        output PRDATA,
+        output PREADY,
+        output PSLVERR
+    );
+endinterface

test/run.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+set -e
+
+../export.py test_regblock.rdl
+
+vlog -sv -suppress 2720 -quiet -f src.f
+vsim -c -quiet tb -do "log -r /*; run -all; exit;"

test/src.f

@@ -0,0 +1,5 @@
+interfaces/apb3_intf.sv
+drivers/apb3_intf_driver.sv
+test_regblock_pkg.sv
+test_regblock.sv
+tb.sv

test/tb.sv

@@ -0,0 +1,51 @@
+module tb;
+    timeunit 1ns;
+    timeprecision 1ps;
+
+    logic rst = '1;
+    logic clk = '0;
+    initial forever begin
+        #10ns;
+        clk = ~clk;
+    end
+
+    apb3_intf apb();
+
+    apb3_intf_driver driver(
+        .clk(clk),
+        .m_apb(apb)
+    );
+
+
+    test_regblock dut (
+        .clk(clk),
+        .rst(rst),
+        .s_apb(apb),
+        .hwif_out()
+    );
+
+
+    initial begin
+        logic [31:0] rd_data;
+
+        repeat(5) @(posedge clk);
+        rst = '0;
+        repeat(5) @(posedge clk);
+
+        driver.read('h000, rd_data);
+        driver.write('h000, 'h0);
+        driver.read('h000, rd_data);
+
+        driver.read('h100, rd_data);
+        driver.write('h100, 'h0);
+        driver.read('h100, rd_data);
+
+        driver.read('h000, rd_data);
+        driver.write('h000, 'hFFFF_FFFF);
+        driver.read('h000, rd_data);
+
+        repeat(5) @(posedge clk);
+        $finish();
+    end
+
+endmodule

test/test_regblock.rdl

@@ -0,0 +1,12 @@
+
+addrmap test_regblock {
+    reg my_reg {
+        field { sw=rw; hw=r; anded;} a[8] = 0x10;
+        field { sw=rw; hw=r; ored;} b[8] = 0x20;
+        field { sw=rw; hw=r; swmod;} c[8] = 0x30;
+    };
+
+    my_reg r0 @0x000;
+    my_reg r1 @0x100;
+    my_reg r2 @0x200;
+};

test/test_regblock.sv

@@ -0,0 +1,372 @@
+// TODO: Add a banner
+module test_regblock (
+        input wire clk,
+        input wire rst,
+
+        apb3_intf.slave s_apb,
+
+        output test_regblock_pkg::test_regblock__out_t hwif_out
+    );
+
+    localparam ADDR_WIDTH = 32;
+    localparam DATA_WIDTH = 32;
+
+    //--------------------------------------------------------------------------
+    // CPU Bus interface logic
+    //--------------------------------------------------------------------------
+    logic cpuif_req;
+    logic cpuif_req_is_wr;
+    logic [ADDR_WIDTH-1:0] cpuif_addr;
+    logic [DATA_WIDTH-1:0] cpuif_wr_data;
+    logic [DATA_WIDTH-1:0] cpuif_wr_bitstrb;
+
+    logic cpuif_rd_ack;
+    logic [DATA_WIDTH-1:0] cpuif_rd_data;
+    logic cpuif_rd_err;
+
+    logic cpuif_wr_ack;
+    logic cpuif_wr_err;
+
+    begin
+        // Request
+        logic is_active;
+        always_ff @(posedge clk) begin
+            if(rst) begin
+                is_active <= '0;
+                cpuif_req <= '0;
+                cpuif_req_is_wr <= '0;
+                cpuif_addr <= '0;
+                cpuif_wr_data <= '0;
+            end else begin
+                if(~is_active) begin
+                    if(s_apb.PSEL) begin
+                        is_active <= '1;
+                        cpuif_req <= '1;
+                        cpuif_req_is_wr <= s_apb.PWRITE;
+                        cpuif_addr <= s_apb.PADDR[ADDR_WIDTH-1:0];
+                        cpuif_wr_data <= s_apb.PWDATA;
+                    end
+                end else begin
+                    cpuif_req <= '0;
+                    if(cpuif_rd_ack || cpuif_wr_ack) begin
+                        is_active <= '0;
+                    end
+                end
+            end
+        end
+        assign cpuif_wr_bitstrb = '0;
+
+        // Response
+        assign s_apb.PREADY = cpuif_rd_ack | cpuif_wr_ack;
+        assign s_apb.PRDATA = cpuif_rd_data;
+        assign s_apb.PSLVERR = cpuif_rd_err | cpuif_wr_err;
+    end
+
+    //--------------------------------------------------------------------------
+    // Address Decode
+    //--------------------------------------------------------------------------
+    typedef struct {
+        logic r0;
+        logic r1;
+        logic r2;
+    } decoded_reg_strb_t;
+    decoded_reg_strb_t decoded_reg_strb;
+    logic decoded_req;
+    logic decoded_req_is_wr;
+    logic [DATA_WIDTH-1:0] decoded_wr_data;
+    logic [DATA_WIDTH-1:0] decoded_wr_bitstrb;
+
+    always_comb begin
+        decoded_reg_strb.r0 = cpuif_req & (cpuif_addr == 'h0);
+        decoded_reg_strb.r1 = cpuif_req & (cpuif_addr == 'h100);
+        decoded_reg_strb.r2 = cpuif_req & (cpuif_addr == 'h200);
+    end
+
+    // Writes are always granted with no error response
+    assign cpuif_wr_ack = cpuif_req & cpuif_req_is_wr;
+    assign cpuif_wr_err = '0;
+
+    // Pass down signals to next stage
+    assign decoded_req = cpuif_req;
+    assign decoded_req_is_wr = cpuif_req_is_wr;
+    assign decoded_wr_data = cpuif_wr_data;
+    assign decoded_wr_bitstrb = cpuif_wr_bitstrb;
+
+    //--------------------------------------------------------------------------
+    // Field logic
+    //--------------------------------------------------------------------------
+    typedef struct {
+        struct {
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } a;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } b;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } c;
+        } r0;
+        struct {
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } a;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } b;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } c;
+        } r1;
+        struct {
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } a;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } b;
+            struct {
+                logic [7:0] next;
+                logic load_next;
+            } c;
+        } r2;
+    } field_combo_t;
+    field_combo_t field_combo;
+
+    typedef struct {
+        struct {
+            logic [7:0] a;
+            logic [7:0] b;
+            logic [7:0] c;
+        } r0;
+        struct {
+            logic [7:0] a;
+            logic [7:0] b;
+            logic [7:0] c;
+        } r1;
+        struct {
+            logic [7:0] a;
+            logic [7:0] b;
+            logic [7:0] c;
+        } r2;
+    } field_storage_t;
+    field_storage_t field_storage;
+
+    // Field: test_regblock.r0.a
+    always_comb begin
+        field_combo.r0.a.next = field_storage.r0.a;
+        field_combo.r0.a.load_next = '0;
+        if(decoded_reg_strb.r0 && decoded_req_is_wr) begin // SW write
+            field_combo.r0.a.next = decoded_wr_data[7:0];
+            field_combo.r0.a.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r0.a <= 'h10;
+        end else if(field_combo.r0.a.load_next) begin
+            field_storage.r0.a <= field_combo.r0.a.next;
+        end
+    end
+    assign hwif_out.r0.a.value = field_storage.r0.a;
+    assign hwif_out.r0.a.anded = &(field_storage.r0.a);
+    // Field: test_regblock.r0.b
+    always_comb begin
+        field_combo.r0.b.next = field_storage.r0.b;
+        field_combo.r0.b.load_next = '0;
+        if(decoded_reg_strb.r0 && decoded_req_is_wr) begin // SW write
+            field_combo.r0.b.next = decoded_wr_data[15:8];
+            field_combo.r0.b.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r0.b <= 'h20;
+        end else if(field_combo.r0.b.load_next) begin
+            field_storage.r0.b <= field_combo.r0.b.next;
+        end
+    end
+    assign hwif_out.r0.b.value = field_storage.r0.b;
+    assign hwif_out.r0.b.ored = |(field_storage.r0.b);
+    // Field: test_regblock.r0.c
+    always_comb begin
+        field_combo.r0.c.next = field_storage.r0.c;
+        field_combo.r0.c.load_next = '0;
+        if(decoded_reg_strb.r0 && decoded_req_is_wr) begin // SW write
+            field_combo.r0.c.next = decoded_wr_data[23:16];
+            field_combo.r0.c.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r0.c <= 'h30;
+        end else if(field_combo.r0.c.load_next) begin
+            field_storage.r0.c <= field_combo.r0.c.next;
+        end
+    end
+    assign hwif_out.r0.c.value = field_storage.r0.c;
+    assign hwif_out.r0.c.swmod = decoded_reg_strb.r0 && decoded_req_is_wr;
+    // Field: test_regblock.r1.a
+    always_comb begin
+        field_combo.r1.a.next = field_storage.r1.a;
+        field_combo.r1.a.load_next = '0;
+        if(decoded_reg_strb.r1 && decoded_req_is_wr) begin // SW write
+            field_combo.r1.a.next = decoded_wr_data[7:0];
+            field_combo.r1.a.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r1.a <= 'h10;
+        end else if(field_combo.r1.a.load_next) begin
+            field_storage.r1.a <= field_combo.r1.a.next;
+        end
+    end
+    assign hwif_out.r1.a.value = field_storage.r1.a;
+    assign hwif_out.r1.a.anded = &(field_storage.r1.a);
+    // Field: test_regblock.r1.b
+    always_comb begin
+        field_combo.r1.b.next = field_storage.r1.b;
+        field_combo.r1.b.load_next = '0;
+        if(decoded_reg_strb.r1 && decoded_req_is_wr) begin // SW write
+            field_combo.r1.b.next = decoded_wr_data[15:8];
+            field_combo.r1.b.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r1.b <= 'h20;
+        end else if(field_combo.r1.b.load_next) begin
+            field_storage.r1.b <= field_combo.r1.b.next;
+        end
+    end
+    assign hwif_out.r1.b.value = field_storage.r1.b;
+    assign hwif_out.r1.b.ored = |(field_storage.r1.b);
+    // Field: test_regblock.r1.c
+    always_comb begin
+        field_combo.r1.c.next = field_storage.r1.c;
+        field_combo.r1.c.load_next = '0;
+        if(decoded_reg_strb.r1 && decoded_req_is_wr) begin // SW write
+            field_combo.r1.c.next = decoded_wr_data[23:16];
+            field_combo.r1.c.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r1.c <= 'h30;
+        end else if(field_combo.r1.c.load_next) begin
+            field_storage.r1.c <= field_combo.r1.c.next;
+        end
+    end
+    assign hwif_out.r1.c.value = field_storage.r1.c;
+    assign hwif_out.r1.c.swmod = decoded_reg_strb.r1 && decoded_req_is_wr;
+    // Field: test_regblock.r2.a
+    always_comb begin
+        field_combo.r2.a.next = field_storage.r2.a;
+        field_combo.r2.a.load_next = '0;
+        if(decoded_reg_strb.r2 && decoded_req_is_wr) begin // SW write
+            field_combo.r2.a.next = decoded_wr_data[7:0];
+            field_combo.r2.a.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r2.a <= 'h10;
+        end else if(field_combo.r2.a.load_next) begin
+            field_storage.r2.a <= field_combo.r2.a.next;
+        end
+    end
+    assign hwif_out.r2.a.value = field_storage.r2.a;
+    assign hwif_out.r2.a.anded = &(field_storage.r2.a);
+    // Field: test_regblock.r2.b
+    always_comb begin
+        field_combo.r2.b.next = field_storage.r2.b;
+        field_combo.r2.b.load_next = '0;
+        if(decoded_reg_strb.r2 && decoded_req_is_wr) begin // SW write
+            field_combo.r2.b.next = decoded_wr_data[15:8];
+            field_combo.r2.b.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r2.b <= 'h20;
+        end else if(field_combo.r2.b.load_next) begin
+            field_storage.r2.b <= field_combo.r2.b.next;
+        end
+    end
+    assign hwif_out.r2.b.value = field_storage.r2.b;
+    assign hwif_out.r2.b.ored = |(field_storage.r2.b);
+    // Field: test_regblock.r2.c
+    always_comb begin
+        field_combo.r2.c.next = field_storage.r2.c;
+        field_combo.r2.c.load_next = '0;
+        if(decoded_reg_strb.r2 && decoded_req_is_wr) begin // SW write
+            field_combo.r2.c.next = decoded_wr_data[23:16];
+            field_combo.r2.c.load_next = '1;
+        end
+    end
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            field_storage.r2.c <= 'h30;
+        end else if(field_combo.r2.c.load_next) begin
+            field_storage.r2.c <= field_combo.r2.c.next;
+        end
+    end
+    assign hwif_out.r2.c.value = field_storage.r2.c;
+    assign hwif_out.r2.c.swmod = decoded_reg_strb.r2 && decoded_req_is_wr;
+
+    //--------------------------------------------------------------------------
+    // Readback
+    //--------------------------------------------------------------------------
+    
+    logic readback_err;
+    logic readback_done;
+    logic [DATA_WIDTH-1:0] readback_data;
+    logic [DATA_WIDTH-1:0] readback_array[3];
+
+    assign readback_array[0][7:0] = (decoded_reg_strb.r0 && !decoded_req_is_wr) ? field_storage.r0.a : '0;
+    assign readback_array[0][15:8] = (decoded_reg_strb.r0 && !decoded_req_is_wr) ? field_storage.r0.b : '0;
+    assign readback_array[0][23:16] = (decoded_reg_strb.r0 && !decoded_req_is_wr) ? field_storage.r0.c : '0;
+    assign readback_array[0][31:24] = '0;
+    assign readback_array[1][7:0] = (decoded_reg_strb.r1 && !decoded_req_is_wr) ? field_storage.r1.a : '0;
+    assign readback_array[1][15:8] = (decoded_reg_strb.r1 && !decoded_req_is_wr) ? field_storage.r1.b : '0;
+    assign readback_array[1][23:16] = (decoded_reg_strb.r1 && !decoded_req_is_wr) ? field_storage.r1.c : '0;
+    assign readback_array[1][31:24] = '0;
+    assign readback_array[2][7:0] = (decoded_reg_strb.r2 && !decoded_req_is_wr) ? field_storage.r2.a : '0;
+    assign readback_array[2][15:8] = (decoded_reg_strb.r2 && !decoded_req_is_wr) ? field_storage.r2.b : '0;
+    assign readback_array[2][23:16] = (decoded_reg_strb.r2 && !decoded_req_is_wr) ? field_storage.r2.c : '0;
+    assign readback_array[2][31:24] = '0;
+
+    always_comb begin
+        automatic logic [DATA_WIDTH-1:0] readback_data_var;
+        readback_done = decoded_req & ~decoded_req_is_wr;
+        readback_err = '0;
+
+        readback_data_var = '0;
+        for(int i=0; i<3; i++) begin
+            readback_data_var |= readback_array[i];
+        end
+        readback_data = readback_data_var;
+    end
+
+    always_ff @(posedge clk) begin
+        if(rst) begin
+            cpuif_rd_ack <= '0;
+            cpuif_rd_data <= '0;
+            cpuif_rd_err <= '0;
+        end else begin
+            cpuif_rd_ack <= readback_done;
+            cpuif_rd_data <= readback_data;
+            cpuif_rd_err <= readback_err;
+        end
+    end
+
+endmodule

test/test_regblock_pkg.sv

@@ -0,0 +1,85 @@
+// TODO: Add a banner
+package test_regblock_pkg;
+    
+    // test_regblock.r0.a
+    typedef struct {
+        logic [7:0] value;
+        logic anded;
+    } test_regblock__r0__a__out_t;
+
+    // test_regblock.r0.b
+    typedef struct {
+        logic [7:0] value;
+        logic ored;
+    } test_regblock__r0__b__out_t;
+
+    // test_regblock.r0.c
+    typedef struct {
+        logic [7:0] value;
+        logic swmod;
+    } test_regblock__r0__c__out_t;
+
+    // test_regblock.r0
+    typedef struct {
+        test_regblock__r0__a__out_t a;
+        test_regblock__r0__b__out_t b;
+        test_regblock__r0__c__out_t c;
+    } test_regblock__r0__out_t;
+
+    // test_regblock.r1.a
+    typedef struct {
+        logic [7:0] value;
+        logic anded;
+    } test_regblock__r1__a__out_t;
+
+    // test_regblock.r1.b
+    typedef struct {
+        logic [7:0] value;
+        logic ored;
+    } test_regblock__r1__b__out_t;
+
+    // test_regblock.r1.c
+    typedef struct {
+        logic [7:0] value;
+        logic swmod;
+    } test_regblock__r1__c__out_t;
+
+    // test_regblock.r1
+    typedef struct {
+        test_regblock__r1__a__out_t a;
+        test_regblock__r1__b__out_t b;
+        test_regblock__r1__c__out_t c;
+    } test_regblock__r1__out_t;
+
+    // test_regblock.r2.a
+    typedef struct {
+        logic [7:0] value;
+        logic anded;
+    } test_regblock__r2__a__out_t;
+
+    // test_regblock.r2.b
+    typedef struct {
+        logic [7:0] value;
+        logic ored;
+    } test_regblock__r2__b__out_t;
+
+    // test_regblock.r2.c
+    typedef struct {
+        logic [7:0] value;
+        logic swmod;
+    } test_regblock__r2__c__out_t;
+
+    // test_regblock.r2
+    typedef struct {
+        test_regblock__r2__a__out_t a;
+        test_regblock__r2__b__out_t b;
+        test_regblock__r2__c__out_t c;
+    } test_regblock__r2__out_t;
+
+    // test_regblock
+    typedef struct {
+        test_regblock__r0__out_t r0;
+        test_regblock__r1__out_t r1;
+        test_regblock__r2__out_t r2;
+    } test_regblock__out_t;
+endpackage

test/wave.do

@@ -0,0 +1,45 @@
+onerror {resume}
+quietly WaveActivateNextPane {} 0
+add wave -noupdate /tb/rst
+add wave -noupdate /tb/clk
+add wave -noupdate /tb/apb/PSEL
+add wave -noupdate /tb/apb/PENABLE
+add wave -noupdate /tb/apb/PWRITE
+add wave -noupdate /tb/apb/PADDR
+add wave -noupdate /tb/apb/PWDATA
+add wave -noupdate /tb/apb/PRDATA
+add wave -noupdate /tb/apb/PREADY
+add wave -noupdate /tb/apb/PSLVERR
+add wave -noupdate -divider DUT
+add wave -noupdate /tb/dut/cpuif_req
+add wave -noupdate /tb/dut/cpuif_req_is_wr
+add wave -noupdate /tb/dut/cpuif_addr
+add wave -noupdate /tb/dut/cpuif_wr_data
+add wave -noupdate /tb/dut/cpuif_wr_bitstrb
+add wave -noupdate /tb/dut/cpuif_rd_ack
+add wave -noupdate /tb/dut/cpuif_rd_data
+add wave -noupdate /tb/dut/cpuif_rd_err
+add wave -noupdate /tb/dut/cpuif_wr_ack
+add wave -noupdate /tb/dut/cpuif_wr_err
+add wave -noupdate -divider Storage
+add wave -noupdate -radix hexadecimal -childformat {{/tb/dut/field_storage.r0 -radix hexadecimal -childformat {{/tb/dut/field_storage.r0.a -radix hexadecimal} {/tb/dut/field_storage.r0.b -radix hexadecimal} {/tb/dut/field_storage.r0.c -radix hexadecimal}}} {/tb/dut/field_storage.r1 -radix hexadecimal -childformat {{/tb/dut/field_storage.r1.a -radix hexadecimal} {/tb/dut/field_storage.r1.b -radix hexadecimal} {/tb/dut/field_storage.r1.c -radix hexadecimal}}} {/tb/dut/field_storage.r2 -radix hexadecimal -childformat {{/tb/dut/field_storage.r2.a -radix hexadecimal} {/tb/dut/field_storage.r2.b -radix hexadecimal} {/tb/dut/field_storage.r2.c -radix hexadecimal}}}} -expand -subitemconfig {/tb/dut/field_storage.r0 {-height 17 -radix hexadecimal -childformat {{/tb/dut/field_storage.r0.a -radix hexadecimal} {/tb/dut/field_storage.r0.b -radix hexadecimal} {/tb/dut/field_storage.r0.c -radix hexadecimal}} -expand} /tb/dut/field_storage.r0.a {-height 17 -radix hexadecimal} /tb/dut/field_storage.r0.b {-height 17 -radix hexadecimal} /tb/dut/field_storage.r0.c {-height 17 -radix hexadecimal} /tb/dut/field_storage.r1 {-height 17 -radix hexadecimal -childformat {{/tb/dut/field_storage.r1.a -radix hexadecimal} {/tb/dut/field_storage.r1.b -radix hexadecimal} {/tb/dut/field_storage.r1.c -radix hexadecimal}} -expand} /tb/dut/field_storage.r1.a {-height 17 -radix hexadecimal} /tb/dut/field_storage.r1.b {-height 17 -radix hexadecimal} /tb/dut/field_storage.r1.c {-height 17 -radix hexadecimal} /tb/dut/field_storage.r2 {-height 17 -radix hexadecimal -childformat {{/tb/dut/field_storage.r2.a -radix hexadecimal} {/tb/dut/field_storage.r2.b -radix hexadecimal} {/tb/dut/field_storage.r2.c -radix hexadecimal}} -expand} /tb/dut/field_storage.r2.a {-height 17 -radix hexadecimal} /tb/dut/field_storage.r2.b {-height 17 -radix hexadecimal} /tb/dut/field_storage.r2.c {-height 17 -radix hexadecimal}} /tb/dut/field_storage
+add wave -noupdate -divider HWIF
+add wave -noupdate -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.a -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.a.value -radix hexadecimal} {/tb/dut/hwif_out.r0.a.anded -radix hexadecimal}}} {/tb/dut/hwif_out.r0.b -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.b.value -radix hexadecimal} {/tb/dut/hwif_out.r0.b.ored -radix hexadecimal}}} {/tb/dut/hwif_out.r0.c -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.c.value -radix hexadecimal} {/tb/dut/hwif_out.r0.c.swmod -radix hexadecimal}}}}} {/tb/dut/hwif_out.r1 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r1.a -radix hexadecimal} {/tb/dut/hwif_out.r1.b -radix hexadecimal} {/tb/dut/hwif_out.r1.c -radix hexadecimal}}} {/tb/dut/hwif_out.r2 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r2.a -radix hexadecimal} {/tb/dut/hwif_out.r2.b -radix hexadecimal} {/tb/dut/hwif_out.r2.c -radix hexadecimal}}}} -expand -subitemconfig {/tb/dut/hwif_out.r0 {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.a -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.a.value -radix hexadecimal} {/tb/dut/hwif_out.r0.a.anded -radix hexadecimal}}} {/tb/dut/hwif_out.r0.b -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.b.value -radix hexadecimal} {/tb/dut/hwif_out.r0.b.ored -radix hexadecimal}}} {/tb/dut/hwif_out.r0.c -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.c.value -radix hexadecimal} {/tb/dut/hwif_out.r0.c.swmod -radix hexadecimal}}}} -expand} /tb/dut/hwif_out.r0.a {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.a.value -radix hexadecimal} {/tb/dut/hwif_out.r0.a.anded -radix hexadecimal}} -expand} /tb/dut/hwif_out.r0.a.value {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r0.a.anded {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r0.b {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.b.value -radix hexadecimal} {/tb/dut/hwif_out.r0.b.ored -radix hexadecimal}} -expand} /tb/dut/hwif_out.r0.b.value {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r0.b.ored {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r0.c {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r0.c.value -radix hexadecimal} {/tb/dut/hwif_out.r0.c.swmod -radix hexadecimal}} -expand} /tb/dut/hwif_out.r0.c.value {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r0.c.swmod {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r1 {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r1.a -radix hexadecimal} {/tb/dut/hwif_out.r1.b -radix hexadecimal} {/tb/dut/hwif_out.r1.c -radix hexadecimal}} -expand} /tb/dut/hwif_out.r1.a {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r1.b {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r1.c {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r2 {-height 17 -radix hexadecimal -childformat {{/tb/dut/hwif_out.r2.a -radix hexadecimal} {/tb/dut/hwif_out.r2.b -radix hexadecimal} {/tb/dut/hwif_out.r2.c -radix hexadecimal}} -expand} /tb/dut/hwif_out.r2.a {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r2.b {-height 17 -radix hexadecimal} /tb/dut/hwif_out.r2.c {-height 17 -radix hexadecimal}} /tb/dut/hwif_out
+TreeUpdate [SetDefaultTree]
+WaveRestoreCursors {{Cursor 1} {650000 ps} 0}
+quietly wave cursor active 1
+configure wave -namecolwidth 150
+configure wave -valuecolwidth 100
+configure wave -justifyvalue left
+configure wave -signalnamewidth 0
+configure wave -snapdistance 10
+configure wave -datasetprefix 0
+configure wave -rowmargin 4
+configure wave -childrowmargin 2
+configure wave -gridoffset 0
+configure wave -gridperiod 1
+configure wave -griddelta 40
+configure wave -timeline 0
+configure wave -timelineunits ps
+update
+WaveRestoreZoom {252900 ps} {755184 ps}

x.rdl

@@ -1,15 +0,0 @@
-
-addrmap top {
-    reg {
-        field {
-            hw=w; sw=rw; precedence=sw;
-        } y = 0;
-
-        field {
-            hw=na; sw=rw;
-        } x = 1;
-
-        y->we;
-        y->hwset;
-    } whee;
-};