prop reference infrastructure, and other things

doc/logbooks/000-Main-Logbook

@@ -27,3 +27,19 @@ the template would do something like:
     {{output_signal(field, "anded")}} = &{{field_value(field)}};
 
 Basically, i'd define a ton of helper functions that return the signal identifier.
+
+================================================================================
+Dev Todo list
+================================================================================
+
+- Lots to do in HWIF layer
+    - reorg base.py+struct.py.
+        - I have no desire to do flattened I/O. No need to split off a base class
+
+
+    - User Signals
+        Generate these in the io struct? I forget what I decided
+    - get_input/output_identifier() functions
+
+- Field logic
+    Basically need all aspects of the dereferencer to be done first

doc/logbooks/Hierarchy-and-Indexing

@@ -0,0 +1,67 @@
+--------------------------------------------------------------------------------
+Preserve Hierarchy
+--------------------------------------------------------------------------------
+I *reaaaally* want to be able to make deferred RDL parameters a reality in the
+future. (https://github.com/SystemRDL/systemrdl-compiler/issues/58)
+
+Proactively design templates to retain "real" hierarchy. This means:
+- Do not flatten/unroll signals. Use SV structs & arrays
+- Do not flatten/unroll logic. Use nested for loops
+
+Sticking to the above should make adding parameter support somewhat less painful.
+
+--------------------------------------------------------------------------------
+Indexing & references
+--------------------------------------------------------------------------------
+Need to define a consistent scheme for referencing hierarchical elements.
+
+When inside a nesting of for loops, and array indexes are intended to increment,
+always use an incrementing indexing scheme when generating iterators:
+    i0, i1, i2, i3, ... i9, i10, i11, etc...
+For example:
+    access_strb.2d_array[i0][i1].array[i3]
+
+Sometimes, an RDL input may create the need to reference an element with
+partially constant indexes.
+For example, given this RDL:
+
+    addrmap top {
+        regfile {
+            reg {
+                field {} f1;
+            } x[8];
+
+            reg {
+                field {} f2;
+            } y;
+
+            y.f2->next = x[3].f1;
+
+        } rf_loop[16];
+    };
+
+The 'next' assignment will have a reference that has the following hierarchcial
+path:
+    top.rf_loop[].x[3].f1
+                |   |
+                |   +--- known index
+                +--- unknown index
+
+It is provable that any RDL references will always follow these truths:
+    - a reference may have a mix of known/unknown indexes in its path
+    - unknown indexes (if any) will always precede known indexes
+    - unknown indexes are not actually part of the relative reference path, and
+      represent replication of the reference.
+      It is impossible for the reference itself to introduce unknown indexes.
+
+When generating SystemVerilog, be sure to generate code such that "unknown" indexes
+are always implicitly known due to the reference being used from within a for loop.
+For example:
+
+    for(int i0=0; i0<16; i0++) begin : rf_loop_array
+        top.rf_loop[i0].y.f2 = top.rf_loop[i0].x[3].f1
+    end
+
+This should be a reasonable thing to accomplish, since unknown indexes should
+only show up in situations where the consumer of the reference is being
+replicated as well, and is therefore implicitly going to be inside a for loop.

doc/logbooks/Signal Dereferencer

@@ -17,3 +17,6 @@ Values:
     If X is a property reference... do whats right...
         my_field->anded === (&path.to.my_field)
     if X is a static value, return the literal
+
+
+See `Hierarchy and Indexing` on details onhow to build path references to stuff

doc/logbooks/Validation Needed

@@ -4,11 +4,12 @@ Things that need validation by the compiler
 ================================================================================
 Many of these are probably already covered, but being paranoid.
 Make a list of things as I think of them.
+Keep them here in case I forget and re-think of them.
 
 Mark these as follows:
     X = Yes, confirmed that the compiler covers this
-    ! = No! Confirmed that the compiler does not check this
+    ! = No! Confirmed that the compiler does not check this, and should.
-    (blank) = TBD
+    ? = TBD
 
 --------------------------------------------------------------------------------
 
@@ -23,11 +24,26 @@ X Field has no knowable value
 
     --> emit a warning?
 
-! multiple field_reset in the same hierarchy
+X References to a component or component property must use unambiguous array indexing
+    For example, if "array_o_regs" is an array...
+    The following is illegal:
+        my_reg.my_field->next = array_o_regs.thing
+        my_reg.my_field->next = array_o_regs.thing->anded
+    This is ok:
+        my_reg.my_field->next = array_o_regs[2].thing
+        my_reg.my_field->next = array_o_regs[2].thing->anded
+
+    NEVERMIND - compiler does not allow indefinite array references at all!
+    References are guaranteed to be unambiguous:
+        "Incompatible number of index dimensions after 'CTRL'. Expected 1, found 0."
+
+X Clause 10.6.1-f (wide registers cannot have access side-effects)
+
+X multiple field_reset in the same hierarchy
     there can only be one signal declared with field_reset
     in a given hierarchy
 
-! multiple cpuif_reset in the same hierarchy
+X multiple cpuif_reset in the same hierarchy
     there can only be one signal declared with cpuif_reset
     in a given hierarchy
 
@@ -39,13 +55,26 @@ X Field has no knowable value
 
     ... or, make these properties clear each-other on assignment
 
+? If a node ispresent=true, and any of it's properties are a reference,
+    then thse references' its ispresent shall also be true
+    Pretty sure this is an explicit clause in the spec.
+    Make sure it is actually implemented
+
+
+? Illegal property references:
+    - reference any of the counter property references to something that isn't a counter
+    - reference hwclr or hwset, but the owner node has them set to False
+        means that the actual inferred signal doesnt exist!
+    - reference swwe/swwel or we/wel, but the owner node has them, AND their complement set to False
+        means that the actual inferred signal doesnt exist!
+
+
 ================================================================================
 Things that need validation by this exporter
 ================================================================================
 List of stuff in case I forget.
-
+    X = Yes! I already implemented this.
     ! = No! exporter does not enforce this yet
-    x = Yes! I already implemented this.
 
 --------------------------------------------------------------------------------
 ! "bridge" addrmap not supported
@@ -56,5 +85,25 @@ cpuif_resets
     ! 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
 
-    ! multiple cpuif_reset
+! async data signals
-        there can be only one cpuif reset
+    Only supporting async signals if they are exclusively used in resets.
+    Anyhting else declared as "async" shall be an error
+    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
+            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
+
+
+! 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

doc/logbooks/template-layers/4-fields

@@ -28,6 +28,16 @@ TODO:
         this may actually only apply to counters...
     This is trivial in a 2-process implementation, but i'd rather avoid the overheads
 
+TODO:
+    Scour the RDL spec.
+    Does this "next state" precedence model hold true in all situations?
+
+TODO:
+    Think about user-extensibility
+    Provide a mechanism for users to extend/override field behavior
+
+TODO:
+    Does the endinness the user sets matter anywhere?
 
 Implementation
     Makes sense to use a listener class

doc/logbooks/template-layers/5-readback-mux

@@ -62,4 +62,8 @@ WARNING:
 
 Forwards response strobe back up to cpu interface layer
 
+TODO:
     Dont forget about alias registers here
+
+TODO:
+    Does the endinness the user sets matter anywhere?

peakrdl/regblock/addr_decode.py

@@ -1,7 +1,7 @@
 import re
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING, List, Union
 
-from systemrdl.node import Node, AddressableNode, RegNode
+from systemrdl.node import Node, AddressableNode, RegNode, FieldNode
 
 
 if TYPE_CHECKING:
@@ -27,6 +27,26 @@ class AddressDecode:
         self._do_address_decode_node(lines, self.top_node)
         return "\n".join(lines)
 
+    def get_access_strobe(self, node: Union[RegNode, FieldNode]) -> str:
+        """
+        Returns the Verilog string that represents the register/field's access strobe.
+        """
+        if isinstance(node, FieldNode):
+            node = node.parent
+
+        path = node.get_rel_path(self.top_node, empty_array_suffix="[!]")
+
+        # replace unknown indexes with incrementing iterators i0, i1, ...
+        class repl:
+            def __init__(self):
+                self.i = 0
+            def __call__(self, match):
+                s = f'i{self.i}'
+                self.i += 1
+                return s
+        path = re.sub(r'!', repl(), path)
+
+        return "decoded_reg_strb." + path
 
     #---------------------------------------------------------------------------
     # Struct generation functions
@@ -55,7 +75,7 @@ class AddressDecode:
         self._indent_level -= 1
 
         if is_top:
-            lines.append(f"{self._indent}}} access_strb_t;")
+            lines.append(f"{self._indent}}} decoded_reg_strb_t;")
         else:
             lines.append(f"{self._indent}}} {node.inst_name}{self._get_node_array_suffix(node)};")
 
@@ -96,26 +116,11 @@ class AddressDecode:
             a += f" + i{i}*'h{stride:x}"
         return a
 
-    def _get_strobe_str(self, node:AddressableNode) -> str:
-        path = node.get_rel_path(self.top_node, array_suffix="[!]", empty_array_suffix="[!]")
-
-        class repl:
-            def __init__(self):
-                self.i = 0
-            def __call__(self, match):
-                s = f'i{self.i}'
-                self.i += 1
-                return s
-
-        path = re.sub(r'!', repl(), path)
-        strb = "access_strb." + path
-        return strb
-
     def _do_address_decode_node(self, lines:List[str], node:AddressableNode) -> None:
         for child in node.children():
             if isinstance(child, RegNode):
                 self._push_array_dims(lines, child)
-                lines.append(f"{self._indent}{self._get_strobe_str(child)} = cpuif_req & (cpuif_addr == {self._get_address_str(child)});")
+                lines.append(f"{self._indent}{self.get_access_strobe(child)} = cpuif_req & (cpuif_addr == {self._get_address_str(child)});")
                 self._pop_array_dims(lines, child)
             elif isinstance(child, AddressableNode):
                 self._push_array_dims(lines, child)

peakrdl/regblock/cpuif/apb4/__init__.py

@@ -16,16 +16,16 @@ class APB4_Cpuif_flattened(APB4_Cpuif):
     def port_declaration(self) -> str:
         # TODO: Reference data/addr width from verilog parameter perhaps?
         lines = [
-            "input wire %s" % self.signal("psel"),
+            "input wire " + self.signal("psel"),
-            "input wire %s" % self.signal("penable"),
+            "input wire " + self.signal("penable"),
-            "input wire %s" % self.signal("pwrite"),
+            "input wire " + self.signal("pwrite"),
-            "input wire %s" % self.signal("pprot"),
+            "input wire " + self.signal("pprot"),
-            "input wire [%d-1:0] %s" % (self.addr_width, self.signal("paddr")),
+            f"input wire [{self.addr_width-1}:0] " + self.signal("paddr"),
-            "input wire [%d-1:0] %s" % (self.data_width, self.signal("pwdata")),
+            f"input wire [{self.data_width-1}:0] " + self.signal("pwdata"),
-            "input wire [%d-1:0] %s" % (self.data_width / 8, self.signal("pstrb")),
+            f"input wire [{(self.data_width / 8)-1}:0] " + self.signal("pstrb"),
-            "output logic %s" % self.signal("pready"),
+            "output logic " + self.signal("pready"),
-            "output logic [%d-1:0] %s" % (self.data_width, self.signal("prdata")),
+            f"output logic [{self.data_width-1}:0] " + self.signal("prdata"),
-            "output logic %s" % self.signal("pslverr"),
+            "output logic " + self.signal("pslverr"),
         ]
         return ",\n".join(lines)
 

peakrdl/regblock/dereferencer.py

@@ -6,21 +6,24 @@ if TYPE_CHECKING:
     from .exporter import RegblockExporter
     from .hwif.base import HwifBase
     from .field_logic import FieldLogic
+    from .addr_decode import AddressDecode
 
 class Dereferencer:
     """
     This class provides an interface to convert conceptual SystemRDL references
     into Verilog identifiers
     """
-    def __init__(self, exporter:'RegblockExporter', hwif:'HwifBase', field_logic: "FieldLogic", top_node:Node):
+    def __init__(self, exporter:'RegblockExporter', top_node:Node, hwif:'HwifBase', address_decode: 'AddressDecode', field_logic: 'FieldLogic'):
         self.exporter = exporter
         self.hwif = hwif
+        self.address_decode = address_decode
         self.field_logic = field_logic
         self.top_node = top_node
 
     def get_value(self, obj: Union[int, FieldNode, SignalNode, PropertyReference]) -> str:
         """
-        Returns the Verilog string that represents the value associated with the object.
+        Returns the Verilog string that represents the readable value associated
+        with the object.
 
         If given a simple scalar value, then the corresponding Verilog literal is returned.
 
@@ -31,7 +34,7 @@ class Dereferencer:
             # Is a simple scalar value
             return f"'h{obj:x}"
 
-        elif isinstance(obj, FieldNode):
+        if isinstance(obj, FieldNode):
             if obj.implements_storage:
                 return self.field_logic.get_storage_identifier(obj)
 
@@ -48,14 +51,14 @@ class Dereferencer:
                 # Fall back to a value of 0
                 return "'h0"
 
-        elif isinstance(obj, SignalNode):
+        if isinstance(obj, SignalNode):
             # Signals are always inputs from the hwif
             return self.hwif.get_input_identifier(obj)
 
-        elif isinstance(obj, PropertyReference):
+        if isinstance(obj, PropertyReference):
-            # TODO: Table G1 describes other possible ref targets
 
-            # Value reduction properties
+            # Value reduction properties.
+            # Wrap with the appropriate Verilog reduction operator
             val = self.get_value(obj.node)
             if obj.name == "anded":
                 return f"&({val})"
@@ -63,15 +66,98 @@ class Dereferencer:
                 return f"|({val})"
             elif obj.name == "xored":
                 return f"^({val})"
-            else:
-                raise RuntimeError
 
+            # references that directly access a property value
+            if obj.name in {
+                'decrvalue',
+                'enable',
+                'haltenable',
+                'haltmask',
+                'hwenable',
+                'hwmask',
+                'incrvalue',
+                'mask',
+                'reset',
+                'resetsignal',
+            }:
+                return self.get_value(obj.node.get_property(obj.name))
+            elif obj.name in {'incr', 'decr'}:
+                prop_value = obj.node.get_property(obj.name)
+                if prop_value is None:
+                    # unset by the user, points to the implied internal signal
+                    raise NotImplementedError # TODO: Implement this
                 else:
-            raise RuntimeError
+                    return self.get_value(prop_value)
+            elif obj.name == "next":
+                prop_value = obj.node.get_property(obj.name)
+                if prop_value is None:
+                    # unset by the user, points to the implied internal signal
+                    raise NotImplementedError # TODO: Implement this
+                else:
+                    return self.get_value(prop_value)
 
-    def get_access_strobe(self, reg: RegNode) -> str:
+            # References to another component value, or an implied input
+            if obj.name in {'hwclr', 'hwset'}:
+                prop_value = obj.node.get_property(obj.name)
+                if prop_value is True:
+                    # Points to inferred hwif input
+                    return self.hwif.get_input_identifier(obj)
+                elif prop_value is False:
+                    # This should never happen, as this is checked by the compiler's validator
+                    raise RuntimeError
+                else:
+                    return self.get_value(prop_value)
+
+            # References to another component value, or an implied input
+            # May have a complementary partner property
+            complementary_pairs = {
+                "we": "wel",
+                "wel": "we",
+                "swwe": "swwel",
+                "swwel": "swwe",
+            }
+            if obj.name in complementary_pairs:
+                prop_value = obj.node.get_property(obj.name)
+                if prop_value is True:
+                    # Points to inferred hwif input
+                    return self.hwif.get_input_identifier(obj)
+                elif prop_value is False:
+                    # Try complementary property
+                    prop_value = obj.node.get_property(complementary_pairs[obj.name])
+                    if prop_value is True:
+                        # Points to inferred hwif input
+                        return f"!({self.hwif.get_input_identifier(obj)})"
+                        raise NotImplementedError # TODO: Implement this
+                    elif prop_value is False:
+                        # This should never happen, as this is checked by the compiler's validator
+                        raise RuntimeError
+                    else:
+                        return f"!({self.get_value(prop_value)})"
+                else:
+                    return self.get_value(prop_value)
+
+            """
+            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_access_strobe(self, obj: Union[RegNode, FieldNode]) -> str:
         """
         Returns the Verilog string that represents the register's access strobe
         """
-        # TODO: Implement me
+        return self.address_decode.get_access_strobe(obj)
-        raise NotImplementedError

peakrdl/regblock/exporter.py

@@ -69,7 +69,7 @@ class RegblockExporter:
         cpuif = cpuif_cls(
             self,
             cpuif_reset=cpuif_reset, # TODO:
-            data_width=32, # TODO:
+            data_width=32, # TODO: derive from the accesswidth used by regs
             addr_width=32 # TODO:
         )
 
@@ -82,7 +82,7 @@ class RegblockExporter:
         address_decode = AddressDecode(self, node)
         field_logic = FieldLogic(self, node)
         readback_mux = ReadbackMux(self, node)
-        dereferencer = Dereferencer(self, hwif, field_logic, node)
+        dereferencer = Dereferencer(self, node, hwif, address_decode, field_logic)
 
         # Build Jinja template context
         context = {

peakrdl/regblock/field_logic/__init__.py

@@ -1,3 +1,4 @@
+import re
 from typing import TYPE_CHECKING, List
 
 from systemrdl.node import Node, AddressableNode, RegNode, FieldNode
@@ -27,10 +28,22 @@ class FieldLogic:
     #---------------------------------------------------------------------------
     # Field utility functions
     #---------------------------------------------------------------------------
-    def get_storage_identifier(self, obj: FieldNode):
+    def get_storage_identifier(self, node: FieldNode):
-        assert obj.implements_storage
+        assert node.implements_storage
 
-        return "TODO: implement get_storage_identifier()"
+        path = node.get_rel_path(self.top_node, empty_array_suffix="[!]")
+
+        # replace unknown indexes with incrementing iterators i0, i1, ...
+        class repl:
+            def __init__(self):
+                self.i = 0
+            def __call__(self, match):
+                s = f'i{self.i}'
+                self.i += 1
+                return s
+        path = re.sub(r'!', repl(), path)
+
+        return "field_storage." + path
 
 
     #---------------------------------------------------------------------------

peakrdl/regblock/hwif/base.py

@@ -1,6 +1,6 @@
 from typing import TYPE_CHECKING, Union
 from systemrdl.node import Node, SignalNode, FieldNode
-from systemrdl.rdltypes import AccessType
+from systemrdl.rdltypes import AccessType, PropertyReference
 
 if TYPE_CHECKING:
     from ..exporter import RegblockExporter
@@ -13,7 +13,7 @@ class HwifBase:
     - Signal inputs (except those that are promoted to the top)
     """
 
-    def __init__(self, exporter:'RegblockExporter', top_node:'Node', package_name:str):
+    def __init__(self, exporter: 'RegblockExporter', top_node: Node, package_name: str):
         self.exporter = exporter
         self.top_node = top_node
         self.package_name = package_name
@@ -41,9 +41,10 @@ class HwifBase:
         Returns True if the object infers an input wire in the hwif
         """
         if isinstance(obj, FieldNode):
-            return obj.get_property("hw") in {AccessType.rw, AccessType.w}
+            return obj.is_hw_writable
         elif isinstance(obj, SignalNode):
-            raise NotImplementedError # TODO:
+            # Signals are implicitly always inputs
+            return True
         else:
             raise RuntimeError
 
@@ -52,29 +53,35 @@ class HwifBase:
         """
         Returns True if the object infers an output wire in the hwif
         """
-        return obj.get_property("hw") in {AccessType.rw, AccessType.r}
+        # TODO: Extend this for signals and prop references?
+        return obj.is_hw_readable
 
 
-    def get_input_identifier(self, obj) -> str:
+    def get_input_identifier(self, obj: Union[FieldNode, SignalNode, PropertyReference]) -> str:
         """
         Returns the identifier string that best represents the input object.
 
         if obj is:
             Field: the fields input value port
             Signal: signal input value
-            TODO: finish this
+            Prop reference:
+                could be an implied hwclr/hwset/swwe/swwel/we/wel input
+                Raise a runtime error if an illegal prop ref is requested, or if
+                the prop ref is not actually implied, but explicitly ref a component
 
+        TODO: finish this
         raises an exception if obj is invalid
         """
         raise NotImplementedError()
 
 
-    def get_output_identifier(self, obj) -> str:
+    def get_output_identifier(self, obj: FieldNode) -> str:
         """
         Returns the identifier string that best represents the output object.
 
         if obj is:
             Field: the fields output value port
+            Property ref: this is also part of the struct
             TODO: finish this
 
         raises an exception if obj is invalid

peakrdl/regblock/hwif/struct.py

@@ -26,7 +26,7 @@ class StructHwif(HwifBase):
         self.has_output_struct = self._do_struct_addressable(lines, self.top_node, is_input=False)
         self._indent_level -= 1
         lines.append("")
-        lines.append(f"endpackage")
+        lines.append("endpackage")
 
         return "\n".join(lines)
 

peakrdl/regblock/module_tmpl.sv

@@ -48,16 +48,26 @@ module {{module_name}} #(
     // Address Decode
     //--------------------------------------------------------------------------
     {{address_decode.get_strobe_struct()|indent}}
-    access_strb_t access_strb;
+    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
         {{address_decode.get_implementation()|indent(8)}}
     end
 
-    // Writes are always posted with no error response
+    // 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
     //--------------------------------------------------------------------------