diff --git a/doc/logbooks/000-Main-Logbook b/doc/logbooks/000-Main-Logbook index 369ef5e..c87b2b8 100644 --- a/doc/logbooks/000-Main-Logbook +++ b/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 diff --git a/doc/logbooks/Hierarchy-and-Indexing b/doc/logbooks/Hierarchy-and-Indexing new file mode 100644 index 0000000..d6da96b --- /dev/null +++ b/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. diff --git a/doc/logbooks/Signal Dereferencer b/doc/logbooks/Signal Dereferencer index 0a40b59..1c626e1 100644 --- a/doc/logbooks/Signal Dereferencer +++ b/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 diff --git a/doc/logbooks/Validation Needed b/doc/logbooks/Validation Needed index b6edbc3..ce9310e 100644 --- a/doc/logbooks/Validation Needed +++ b/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 - (blank) = TBD + ! = No! Confirmed that the compiler does not check this, and should. + ? = 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 - there can be only one cpuif reset +! async data signals + 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 diff --git a/doc/logbooks/template-layers/4-fields b/doc/logbooks/template-layers/4-fields index 85993f5..a2b4fee 100644 --- a/doc/logbooks/template-layers/4-fields +++ b/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 diff --git a/doc/logbooks/template-layers/5-readback-mux b/doc/logbooks/template-layers/5-readback-mux index 9284446..8fe2f43 100644 --- a/doc/logbooks/template-layers/5-readback-mux +++ b/doc/logbooks/template-layers/5-readback-mux @@ -62,4 +62,8 @@ WARNING: Forwards response strobe back up to cpu interface layer -Dont forget about alias registers here +TODO: + Dont forget about alias registers here + +TODO: + Does the endinness the user sets matter anywhere? diff --git a/peakrdl/regblock/addr_decode.py b/peakrdl/regblock/addr_decode.py index 24361d9..d7993d8 100644 --- a/peakrdl/regblock/addr_decode.py +++ b/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) diff --git a/peakrdl/regblock/cpuif/apb4/__init__.py b/peakrdl/regblock/cpuif/apb4/__init__.py index dcd5cda..7eba696 100644 --- a/peakrdl/regblock/cpuif/apb4/__init__.py +++ b/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 %s" % self.signal("penable"), - "input wire %s" % self.signal("pwrite"), - "input wire %s" % self.signal("pprot"), - "input wire [%d-1:0] %s" % (self.addr_width, self.signal("paddr")), - "input wire [%d-1:0] %s" % (self.data_width, self.signal("pwdata")), - "input wire [%d-1:0] %s" % (self.data_width / 8, self.signal("pstrb")), - "output logic %s" % self.signal("pready"), - "output logic [%d-1:0] %s" % (self.data_width, self.signal("prdata")), - "output logic %s" % self.signal("pslverr"), + "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"), ] return ",\n".join(lines) diff --git a/peakrdl/regblock/dereferencer.py b/peakrdl/regblock/dereferencer.py index 39c7c8a..d393bc9 100644 --- a/peakrdl/regblock/dereferencer.py +++ b/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): - # TODO: Table G1 describes other possible ref targets + if isinstance(obj, PropertyReference): - # 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 - 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: + 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 - raise NotImplementedError + return self.address_decode.get_access_strobe(obj) diff --git a/peakrdl/regblock/exporter.py b/peakrdl/regblock/exporter.py index 3b23aa1..6d1d98c 100644 --- a/peakrdl/regblock/exporter.py +++ b/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 = { diff --git a/peakrdl/regblock/field_logic/__init__.py b/peakrdl/regblock/field_logic/__init__.py index 719af96..2f9bc3a 100644 --- a/peakrdl/regblock/field_logic/__init__.py +++ b/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): - assert obj.implements_storage + def get_storage_identifier(self, node: FieldNode): + 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 #--------------------------------------------------------------------------- diff --git a/peakrdl/regblock/hwif/base.py b/peakrdl/regblock/hwif/base.py index 83f92fd..ee4624d 100644 --- a/peakrdl/regblock/hwif/base.py +++ b/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 diff --git a/peakrdl/regblock/hwif/struct.py b/peakrdl/regblock/hwif/struct.py index b778080..a5253d3 100644 --- a/peakrdl/regblock/hwif/struct.py +++ b/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) diff --git a/peakrdl/regblock/module_tmpl.sv b/peakrdl/regblock/module_tmpl.sv index 5b9cb32..787be92 100644 --- a/peakrdl/regblock/module_tmpl.sv +++ b/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 //--------------------------------------------------------------------------