From 0d82154b9d88444276f3faa2c8d0625c0cad09bb Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Mon, 15 May 2023 22:53:17 -0700 Subject: [PATCH] Add support for field paritycheck. #35 --- .gitignore | 1 + docs/index.rst | 2 +- docs/props/field.rst | 10 ++++- src/peakrdl_regblock/__about__.py | 2 +- src/peakrdl_regblock/exporter.py | 5 +++ src/peakrdl_regblock/field_logic/__init__.py | 13 +++++++ .../field_logic/generators.py | 5 +++ .../field_logic/templates/field_storage.sv | 14 +++++++ src/peakrdl_regblock/module_tmpl.sv | 21 +++++++++++ src/peakrdl_regblock/parity.py | 34 +++++++++++++++++ src/peakrdl_regblock/scan_design.py | 11 ++++++ tests/lib/tb_base.sv | 12 ++++++ tests/test_parity/__init__.py | 0 tests/test_parity/regblock.rdl | 14 +++++++ tests/test_parity/tb_template.sv | 37 +++++++++++++++++++ tests/test_parity/testcase.py | 5 +++ 16 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 src/peakrdl_regblock/parity.py create mode 100644 tests/test_parity/__init__.py create mode 100644 tests/test_parity/regblock.rdl create mode 100644 tests/test_parity/tb_template.sv create mode 100644 tests/test_parity/testcase.py diff --git a/.gitignore b/.gitignore index 79b5899..7278eb0 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ **/*.log **/*.pb **/.Xil +**/.coverage.* build/ dist/ diff --git a/docs/index.rst b/docs/index.rst index be9e00e..7e7ed7b 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -15,7 +15,7 @@ your hardware design. .. warning:: The PeakRDL-regblock SV generator is still in pre-production (v0.x version numbers). - During this time, I may decide to refactor things which could break compatibility. + During this time, I may decide to refactor things which could affect compatibility. Installing diff --git a/docs/props/field.rst b/docs/props/field.rst index cd90298..fc8e35a 100644 --- a/docs/props/field.rst +++ b/docs/props/field.rst @@ -460,7 +460,15 @@ If assigned, replaces the inferred ``hwif_in..next`` input with an explicit refe paritycheck ^^^^^^^^^^^ -|NO| +If set, enables parity checking for this field. + +Adds a ``parity_error`` output signal to the module. + +.. note:: + + If this field does not implement storage, the ``partycheck`` property is ignored. + + precedence ^^^^^^^^^^ diff --git a/src/peakrdl_regblock/__about__.py b/src/peakrdl_regblock/__about__.py index 8ad8ffb..54fd93a 100644 --- a/src/peakrdl_regblock/__about__.py +++ b/src/peakrdl_regblock/__about__.py @@ -1 +1 @@ -__version__ = "0.14.0-rc1" +__version__ = "0.14.0-rc2" diff --git a/src/peakrdl_regblock/exporter.py b/src/peakrdl_regblock/exporter.py index c92f7f0..5846f5d 100644 --- a/src/peakrdl_regblock/exporter.py +++ b/src/peakrdl_regblock/exporter.py @@ -19,6 +19,7 @@ from .hwif import Hwif from .write_buffering import WriteBuffering from .read_buffering import ReadBuffering from .external_acks import ExternalWriteAckGenerator, ExternalReadAckGenerator +from .parity import ParityErrorReduceGenerator if TYPE_CHECKING: from systemrdl.node import SignalNode @@ -153,6 +154,7 @@ class RegblockExporter: self.dereferencer = Dereferencer(self) ext_write_acks = ExternalWriteAckGenerator(self) ext_read_acks = ExternalReadAckGenerator(self) + parity = ParityErrorReduceGenerator(self) # Validate that there are no unsupported constructs DesignValidator(self).do_validate() @@ -176,6 +178,7 @@ class RegblockExporter: "readback_implementation": readback_implementation, "ext_write_acks": ext_write_acks, "ext_read_acks": ext_read_acks, + "parity": parity, "get_always_ff_event": self.dereferencer.get_always_ff_event, "ds": self.ds, "kwf": kwf, @@ -243,6 +246,8 @@ class DesignState: self.has_external_block = False self.has_external_addressable = False + self.has_paritycheck = False + # Track any referenced enums self.user_enums = [] # type: List[Type[UserEnum]] diff --git a/src/peakrdl_regblock/field_logic/__init__.py b/src/peakrdl_regblock/field_logic/__init__.py index 8ab9f73..06f876b 100644 --- a/src/peakrdl_regblock/field_logic/__init__.py +++ b/src/peakrdl_regblock/field_logic/__init__.py @@ -274,6 +274,19 @@ class FieldLogic: # Not sw modifiable return "1'b0" + def get_parity_identifier(self, field: 'FieldNode') -> str: + """ + Returns the identifier for the stored 'golden' parity value of the field + """ + path = get_indexed_path(self.top_node, field) + return f"field_storage.{path}.parity" + + def get_parity_error_identifier(self, field: 'FieldNode') -> str: + """ + Returns the identifier for whether the field currently has a parity error + """ + path = get_indexed_path(self.top_node, field) + return f"field_combo.{path}.parity_error" def has_next_q(self, field: 'FieldNode') -> bool: """ diff --git a/src/peakrdl_regblock/field_logic/generators.py b/src/peakrdl_regblock/field_logic/generators.py index 380c5d1..fc7f477 100644 --- a/src/peakrdl_regblock/field_logic/generators.py +++ b/src/peakrdl_regblock/field_logic/generators.py @@ -53,6 +53,8 @@ class CombinationalStructGenerator(RDLStructGenerator): self.add_up_counter_members(node) if node.is_down_counter: self.add_down_counter_members(node) + if node.get_property('paritycheck'): + self.add_member("parity_error") self.pop_struct() def add_up_counter_members(self, node: 'FieldNode') -> None: @@ -88,6 +90,8 @@ class FieldStorageStructGenerator(RDLStructGenerator): if node.implements_storage: self.add_member("value", node.width) + if node.get_property('paritycheck'): + self.add_member("parity") if self.field_logic.has_next_q(node): self.add_member("next_q", node.width) @@ -233,6 +237,7 @@ class FieldLogicGenerator(RDLForLoopGenerator): 'get_value': self.exp.dereferencer.get_value, 'get_resetsignal': self.exp.dereferencer.get_resetsignal, 'get_input_identifier': self.exp.hwif.get_input_identifier, + 'ds': self.ds, } self.add_content(self.field_storage_template.render(context)) diff --git a/src/peakrdl_regblock/field_logic/templates/field_storage.sv b/src/peakrdl_regblock/field_logic/templates/field_storage.sv index 971cc0a..431fbc6 100644 --- a/src/peakrdl_regblock/field_logic/templates/field_storage.sv +++ b/src/peakrdl_regblock/field_logic/templates/field_storage.sv @@ -3,6 +3,7 @@ always_comb begin automatic logic [{{node.width-1}}:0] next_c = {{field_logic.get_storage_identifier(node)}}; automatic logic load_next_c = '0; + {%- for signal in extra_combo_signals %} {{field_logic.get_field_combo_identifier(node, signal.name)}} = {{signal.default_assignment}}; {%- endfor %} @@ -13,22 +14,35 @@ always_comb begin {%- endfor %} end {%- endfor %} + {%- if node.is_up_counter %} {{counter_macros.up_counter(node)}} {%- endif %} + {%- if node.is_down_counter %} {{counter_macros.down_counter(node)}} {%- endif %} {{field_logic.get_field_combo_identifier(node, "next")}} = next_c; {{field_logic.get_field_combo_identifier(node, "load_next")}} = load_next_c; + + {%- if node.get_property('paritycheck') %} + {{field_logic.get_parity_error_identifier(node)}} = ({{field_logic.get_parity_identifier(node)}} != ^{{field_logic.get_storage_identifier(node)}}); + {%- endif %} end always_ff {{get_always_ff_event(resetsignal)}} begin {% if reset is not none -%} if({{get_resetsignal(resetsignal)}}) begin {{field_logic.get_storage_identifier(node)}} <= {{reset}}; + {%- if node.get_property('paritycheck') %} + {{field_logic.get_parity_identifier(node)}} <= ^{{reset}}; + {%- endif %} end else {% endif %}if({{field_logic.get_field_combo_identifier(node, "load_next")}}) begin {{field_logic.get_storage_identifier(node)}} <= {{field_logic.get_field_combo_identifier(node, "next")}}; + {%- if node.get_property('paritycheck') %} + {{field_logic.get_parity_identifier(node)}} <= ^{{field_logic.get_field_combo_identifier(node, "next")}}; + {%- endif %} end + {%- if field_logic.has_next_q(node) %} {{field_logic.get_next_q_identifier(node)}} <= {{get_input_identifier(node)}}; {%- endif %} diff --git a/src/peakrdl_regblock/module_tmpl.sv b/src/peakrdl_regblock/module_tmpl.sv index 0d91b33..759a954 100644 --- a/src/peakrdl_regblock/module_tmpl.sv +++ b/src/peakrdl_regblock/module_tmpl.sv @@ -13,6 +13,11 @@ module {{ds.module_name}} ( {%- endif %} {%- endfor %} + {%- if ds.has_paritycheck %} + + output logic parity_error, + {%- endif %} + {{cpuif.port_declaration|indent(8)}} {%- if hwif.has_input_struct or hwif.has_output_struct %},{% endif %} @@ -168,6 +173,22 @@ module {{ds.module_name}} ( {{field_logic.get_implementation()|indent}} +{%- if ds.has_paritycheck %} + + //-------------------------------------------------------------------------- + // Parity Error + //-------------------------------------------------------------------------- + always_ff {{get_always_ff_event(cpuif.reset)}} begin + if({{get_resetsignal(cpuif.reset)}}) begin + parity_error <= '0; + end else begin + automatic logic err = '0; + {{parity.get_implementation()|indent(12)}} + parity_error <= err; + end + end +{%- endif %} + {%- if ds.has_buffered_read_regs %} //-------------------------------------------------------------------------- diff --git a/src/peakrdl_regblock/parity.py b/src/peakrdl_regblock/parity.py new file mode 100644 index 0000000..980a2c7 --- /dev/null +++ b/src/peakrdl_regblock/parity.py @@ -0,0 +1,34 @@ +from typing import TYPE_CHECKING + +from systemrdl.walker import WalkerAction + + +from .forloop_generator import RDLForLoopGenerator + +if TYPE_CHECKING: + from .exporter import RegblockExporter + from systemrdl.node import FieldNode, AddressableNode + + +class ParityErrorReduceGenerator(RDLForLoopGenerator): + def __init__(self, exp: 'RegblockExporter') -> None: + super().__init__() + self.exp = exp + + def get_implementation(self) -> str: + content = self.get_content(self.exp.ds.top_node) + if content is None: + return "" + return content + + def enter_AddressableComponent(self, node: 'AddressableNode') -> WalkerAction: + super().enter_AddressableComponent(node) + if node.external: + return WalkerAction.SkipDescendants + return WalkerAction.Continue + + def enter_Field(self, node: 'FieldNode') -> None: + if node.get_property('paritycheck') and node.implements_storage: + self.add_content( + f"err |= {self.exp.field_logic.get_parity_error_identifier(node)};" + ) diff --git a/src/peakrdl_regblock/scan_design.py b/src/peakrdl_regblock/scan_design.py index 4a19190..9992245 100644 --- a/src/peakrdl_regblock/scan_design.py +++ b/src/peakrdl_regblock/scan_design.py @@ -106,3 +106,14 @@ class DesignScanner(RDLListener): def enter_Field(self, node: 'FieldNode') -> None: if node.is_sw_writable and (node.msb < node.lsb): self.ds.has_writable_msb0_fields = True + + if node.get_property('paritycheck') and node.implements_storage: + self.ds.has_paritycheck = True + + if node.get_property('reset') is None: + self.msg.warning( + f"Field '{node.inst_name}' includes parity check logic, but " + "its reset value was not defined. Will result in an undefined " + "value on the module's 'parity_error' output.", + self.top_node.inst.property_src_ref.get('paritycheck', self.top_node.inst.inst_src_ref) + ) diff --git a/tests/lib/tb_base.sv b/tests/lib/tb_base.sv index a0b7442..7d7363e 100644 --- a/tests/lib/tb_base.sv +++ b/tests/lib/tb_base.sv @@ -26,6 +26,11 @@ module tb; regblock_pkg::regblock__out_t hwif_out; {%- endif %} +{%- if exporter.ds.has_paritycheck %} + logic parity_error; +{%- endif %} + + {%- block declarations %} {%- endblock %} @@ -43,6 +48,10 @@ module tb; input hwif_out; {%- endif %} +{%- if exporter.ds.has_paritycheck %} + input parity_error; +{%- endif %} + {%- filter indent %} {%- block clocking_dirs %} {%- endblock %} @@ -68,6 +77,9 @@ module tb; ##1; tmp = {>>{hwif_out}}; // Workaround for Xilinx's xsim - assign to tmp variable if(!rst) assert(!$isunknown(tmp)) else $error("hwif_out has X's!"); + {%- if exporter.ds.has_paritycheck %} + if(!rst) assert(!$isunknown(parity_error)) else $error("parity_error has X's!"); + {%- endif %} end end {%- endif %} diff --git a/tests/test_parity/__init__.py b/tests/test_parity/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_parity/regblock.rdl b/tests/test_parity/regblock.rdl new file mode 100644 index 0000000..7056baf --- /dev/null +++ b/tests/test_parity/regblock.rdl @@ -0,0 +1,14 @@ +addrmap top { + default paritycheck; + default sw=rw; + default hw=na; + + reg my_reg { + field {} f1[16] = 0; + field {} f2[8] = 0; + field {} f3 = 0; + }; + + my_reg r1 @ 0x000; + my_reg r2[8] @ 0x1000; +}; diff --git a/tests/test_parity/tb_template.sv b/tests/test_parity/tb_template.sv new file mode 100644 index 0000000..3309500 --- /dev/null +++ b/tests/test_parity/tb_template.sv @@ -0,0 +1,37 @@ +{% extends "lib/tb_base.sv" %} + +{% block seq %} + {% sv_line_anchor %} + ##1; + cb.rst <= '0; + ##1; + + fork + begin + repeat(50) begin + automatic int i = $urandom_range(7,0); + cpuif.write('h0, $urandom()); + cpuif.write('h1000 + i*4, $urandom()); + end + end + begin + forever begin + assert(cb.parity_error != 1'b1); + @cb; + end + end + join_any + disable fork; + + cpuif.write('h0, 'd0); + force dut.field_storage.r1.f1.value[3] = 1'b1; + release dut.field_storage.r1.f1.value[3]; + @cb; + @cb; + assert(cb.parity_error == 1'b1); + cpuif.write('h0, 'd0); + @cb; + @cb; + assert(cb.parity_error == 1'b0); + +{% endblock %} diff --git a/tests/test_parity/testcase.py b/tests/test_parity/testcase.py new file mode 100644 index 0000000..835b5ef --- /dev/null +++ b/tests/test_parity/testcase.py @@ -0,0 +1,5 @@ +from ..lib.sim_testcase import SimTestCase + +class Test(SimTestCase): + def test_dut(self): + self.run_test()