From 9295cbb7c0850c0f63ed592795c72cda77b33f4d Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Mon, 28 Feb 2022 22:05:24 -0800 Subject: [PATCH] Add precedence test. fixup docs --- README.md | 4 -- docs/dev_notes/Validation Needed | 42 +++++++++--------- docs/hwif.rst | 2 +- docs/index.rst | 9 +--- docs/limitations.rst | 12 ++++++ docs/props/field.rst | 2 +- docs/props/signal.rst | 8 ++-- peakrdl/regblock/__about__.py | 2 +- peakrdl/regblock/scan_design.py | 67 ++++++++++++++++++++++++----- test/README.md | 14 ++++-- test/test_precedence/__init__.py | 0 test/test_precedence/regblock.rdl | 26 +++++++++++ test/test_precedence/tb_template.sv | 26 +++++++++++ test/test_precedence/testcase.py | 5 +++ 14 files changed, 164 insertions(+), 55 deletions(-) create mode 100644 test/test_precedence/__init__.py create mode 100644 test/test_precedence/regblock.rdl create mode 100644 test/test_precedence/tb_template.sv create mode 100644 test/test_precedence/testcase.py diff --git a/README.md b/README.md index 342f252..3aef6c4 100644 --- a/README.md +++ b/README.md @@ -2,10 +2,6 @@ [![build](https://github.com/SystemRDL/PeakRDL-regblock/workflows/build/badge.svg)](https://github.com/SystemRDL/PeakRDL-regblock/actions?query=workflow%3Abuild+branch%3Amain) [![PyPI - Python Version](https://img.shields.io/pypi/pyversions/peakrdl-regblock.svg)](https://pypi.org/project/peakrdl-regblock) -# IMPORTANT - -This project has no official releases yet and is still under active development! - # PeakRDL-regblock Generate SystemVerilog RTL that implements a register block from compiled SystemRDL input. diff --git a/docs/dev_notes/Validation Needed b/docs/dev_notes/Validation Needed index 23b20cd..5c2ac3d 100644 --- a/docs/dev_notes/Validation Needed +++ b/docs/dev_notes/Validation Needed @@ -142,10 +142,29 @@ X Warn/error on any signal with cpuif_reset set, that is not in the top-level addrmap. At the very least, warn that it will be ignored -! "bridge" addrmap not supported +X "bridge" addrmap not supported export shall refuse to process an addrmap marked as a "bridge" Only need to check top-level. Compiler will enforce that child nodes arent bridges +X regwidth/accesswidth is sane + X accesswidth == regwidth + Enforce this for now. Dont feel like supporting fancy modes yet + X regwidth < accesswidth + This is illegal and is enforced by the compiler. + X regwidth > accesswidth + Need to extend address decode strobes to have multiple bits + this is where looking at endianness matters to determine field placement + Dont feel like supporting this yet + X constant regwidth? + For now, probably limit to only allow the same regwidth everywhere? + + +X Do not allow unaligned addresses + All offsets & strides shall be a multiple of the regwidth used + + X each reg needs to be aligned to its width + X each regfile/addrmap/stride shall be aligned to the largest regwidth it encloses + ! async data signals Only supporting async signals if they are exclusively used in resets. Anything else declared as "async" shall emit a warning that it is ignored @@ -154,27 +173,6 @@ X Warn/error on any signal with cpuif_reset set, that is not in the top-level ! Error if a property references a non-signal component, or property reference from outside the export hierarchy -! regwidth/accesswidth is sane - ! accesswidth == regwidth - Enforce this for now. Dont feel like supporting fancy modes yet - X regwidth < accesswidth - This is illegal and is enforced by the compiler. - ! regwidth > accesswidth - Need to extend address decode strobes to have multiple bits - this is where looking at endinaness matters to determine field placement - Dont feel like supporting this yet - ! constant regwidth? - For now, probably limit to only allow the same regwidth everywhere? - - -! Do not allow unaligned addresses - All offsets & strides shall be a multiple of the regwidth used - - - each reg needs to be aligned to its width - - each regfile/addrmap/stride shall be aligned to the largest regwidth it encloses - --> Should i promote this check to the compiler? At least as a warnable condition - Currently i think I only do the more stringent case of block alignment. - ! Add warning for sticky race condition stickybit and other similar situations generally should use hw precedence. Emit a warning as appropriate diff --git a/docs/hwif.rst b/docs/hwif.rst index d7f3ada..c0af714 100644 --- a/docs/hwif.rst +++ b/docs/hwif.rst @@ -48,4 +48,4 @@ For example, a simple design such as: hwif_in.my_reg[1].my_field.we For brevity in this documentation, hwif features will be described using shorthand -notation that omits the hierarchcal path: ``hwif_out..`` +notation that omits the hierarchical path: ``hwif_out..`` diff --git a/docs/index.rst b/docs/index.rst index 8391e6e..9f67b50 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -2,7 +2,7 @@ Introduction ============ PeakRDL-regblock is a free and open-source control & status register (CSR) compiler. -This code generator that will translate your SystemRDL register descripton into +This code generator that will translate your SystemRDL register description into a synthesizable SystemVerilog RTL module that can be easily instantiated into your hardware design. @@ -16,16 +16,11 @@ your hardware design. Installing ---------- -.. important:: - - This project has no official releases yet and is still under active development! - Install from `PyPi`_ using pip .. code-block:: bash - # NOT RELEASED YET - # python3 -m pip install peakrdl-regblock + python3 -m pip install peakrdl-regblock .. _PyPi: https://pypi.org/project/peakrdl-regblock diff --git a/docs/limitations.rst b/docs/limitations.rst index 68e768a..72ce7a7 100644 --- a/docs/limitations.rst +++ b/docs/limitations.rst @@ -29,3 +29,15 @@ No partial writes Some protocols describe byte-level write strobes. These are not supported yet. All write transfers must access the entire register width. + + +Register width, Access width and CPUIF bus width +------------------------------------------------ +To keep the initial architecture simpler, currently ``regwidth``, ``accesswidth`` +and the resulting CPU bus width has some limitations: + +* All registers shall have ``regwidth`` == ``accesswidth`` +* ``regwidth`` shall be the same across all registers within the block being exported. +* The CPU interface's bus width is statically determined by the ``regwidth`` used. + +I have plans to remove these restrictions and allow for more flexibility in the future. diff --git a/docs/props/field.rst b/docs/props/field.rst index 8f9053c..e1baf44 100644 --- a/docs/props/field.rst +++ b/docs/props/field.rst @@ -533,7 +533,7 @@ paritycheck precedence ^^^^^^^^^^ -|EX| +|OK| reset ^^^^^ diff --git a/docs/props/signal.rst b/docs/props/signal.rst index 4850288..7f7d453 100644 --- a/docs/props/signal.rst +++ b/docs/props/signal.rst @@ -7,19 +7,19 @@ Signal Properties activehigh/activelow -------------------- -|EX| +|OK| sync/async ---------- -|EX| +|OK| Only supported for signals used as resets to infer edge-sensitive reset. Ignored in all other contexts. cpuif_reset ----------- -|EX| +|OK| field_reset ----------- -|EX| +|OK| diff --git a/peakrdl/regblock/__about__.py b/peakrdl/regblock/__about__.py index b1e19fe..3dc1f76 100644 --- a/peakrdl/regblock/__about__.py +++ b/peakrdl/regblock/__about__.py @@ -1 +1 @@ -__version__ = "0.1.0-a1" +__version__ = "0.1.0" diff --git a/peakrdl/regblock/scan_design.py b/peakrdl/regblock/scan_design.py index 174af9f..4a33ad0 100644 --- a/peakrdl/regblock/scan_design.py +++ b/peakrdl/regblock/scan_design.py @@ -1,11 +1,11 @@ -from typing import TYPE_CHECKING, Set +from typing import TYPE_CHECKING, Set, List from collections import OrderedDict from systemrdl.walker import RDLListener, RDLWalker -from systemrdl.node import AddrmapNode, SignalNode +from systemrdl.node import SignalNode, AddressableNode if TYPE_CHECKING: - from systemrdl.node import Node, RegNode, MemNode, FieldNode + from systemrdl.node import Node, RegNode, FieldNode from .exporter import RegblockExporter @@ -21,6 +21,9 @@ class DesignScanner(RDLListener): self.cpuif_data_width = 0 self.msg = exp.top_node.env.msg + # Keep track of max regwidth encountered in a given block + self.max_regwidth_stack = [] # type: List[int] + # Collections of signals that were actually referenced by the design self.in_hier_signal_paths = set() # type: Set[str] self.out_of_hier_signals = OrderedDict() # type: OrderedDict[str, SignalNode] @@ -49,6 +52,14 @@ class DesignScanner(RDLListener): # collect out-of-hier field_reset, if any self._get_out_of_hier_field_reset() + # Ensure addrmap is not a bridge. This concept does not make sense for + # terminal components. + if self.exp.top_node.get_property('bridge'): + self.msg.error( + "Regblock generator does not support exporting bridge address maps", + self.exp.top_node.inst.property_src_ref.get('bridge', self.exp.top_node.inst.inst_src_ref) + ) + RDLWalker().walk(self.exp.top_node, self) if self.msg.had_error: self.msg.fatal( @@ -57,11 +68,51 @@ class DesignScanner(RDLListener): raise ValueError def enter_Reg(self, node: 'RegNode') -> None: + regwidth = node.get_property('regwidth') + + self.max_regwidth_stack[-1] = max(self.max_regwidth_stack[-1], regwidth) + # The CPUIF's bus width is sized according to the largest register in the design - self.cpuif_data_width = max(self.cpuif_data_width, node.get_property('regwidth')) + # TODO: make this user-overridable once more flexible regwidth/accesswidths are supported + self.cpuif_data_width = max(self.cpuif_data_width, regwidth) + + # TODO: remove this limitation eventually + if regwidth != self.cpuif_data_width: + self.msg.error( + "register blocks with non-uniform regwidths are not supported yet", + node.inst.property_src_ref.get('regwidth', node.inst.inst_src_ref) + ) + + # TODO: remove this limitation eventually + if regwidth != node.get_property('accesswidth'): + self.msg.error( + "Registers that have an accesswidth different from the register width are not supported yet", + node.inst.property_src_ref.get('accesswidth', node.inst.inst_src_ref) + ) + + def enter_AddressableComponent(self, node: AddressableNode) -> None: + self.max_regwidth_stack.append(0) + + def exit_AddressableComponent(self, node: AddressableNode) -> None: + max_block_regwidth = self.max_regwidth_stack.pop() + if self.max_regwidth_stack: + self.max_regwidth_stack[-1] = max(self.max_regwidth_stack[-1], max_block_regwidth) + + alignment = int(max_block_regwidth / 8) + if (node.raw_address_offset % alignment) != 0: + self.msg.error( + f"Unaligned registers are not supported. Address offset of instance '{node.inst_name}' must be a multiple of {alignment}", + node.inst.inst_src_ref + ) + + if node.is_array and (node.array_stride % alignment) != 0: + self.msg.error( + f"Unaligned registers are not supported. Address stride of instance array '{node.inst_name}' must be a multiple of {alignment}", + node.inst.inst_src_ref + ) def enter_Component(self, node: 'Node') -> None: - if not isinstance(node, AddrmapNode) and node.external: + if node.external and (node != self.exp.top_node): self.msg.error( "Exporter does not support external components", node.inst.inst_src_ref @@ -93,9 +144,3 @@ class DesignScanner(RDLListener): self.out_of_hier_signals[path] = value else: self.in_hier_signal_paths.add(path) - - def enter_Mem(self, node: 'MemNode') -> None: - self.msg.error( - "Cannot export a register block that contains a memory", - node.inst.inst_src_ref - ) diff --git a/test/README.md b/test/README.md index a656e71..6e1b8d8 100644 --- a/test/README.md +++ b/test/README.md @@ -6,10 +6,16 @@ Testcases require an installation of the Questa simulator, and for `vlog` & `vsim` commands to be visible via the PATH environment variable. -*Questa - Intel FPGA Starter Edition* can be downloaded for free from -https://fpgasoftware.intel.com/ and is sufficient to run unit tests. You will need -to generate a free license file to unlock the software: https://licensing.intel.com/psg/s/sales-signup-evaluationlicenses - +*Questa - Intel FPGA Starter Edition* can be downloaded for free from Intel: +* Go to https://www.intel.com/content/www/us/en/collections/products/fpga/software/downloads.html?edition=pro&s=Newest +* Select latest version of *Intel Quartus Prime Pro* +* Go to the *Individual Files* tab. +* Download Questa files. (Don't forget part 2!) +* Install +* Go to https://licensing.intel.com/psg/s/sales-signup-evaluationlicenses +* Generate a free *Starter Edition* license file for Questa + * Easiest to use a *fixed* license using your NIC ID (MAC address of your network card via `ifconfig`) +* Download the license file and point the `LM_LICENSE_FILE` environment variable to the folder which contains it. ## Vivado (optional) diff --git a/test/test_precedence/__init__.py b/test/test_precedence/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/test_precedence/regblock.rdl b/test/test_precedence/regblock.rdl new file mode 100644 index 0000000..8affd3e --- /dev/null +++ b/test/test_precedence/regblock.rdl @@ -0,0 +1,26 @@ +addrmap top { + reg { + field { + sw=rw; + hw=w; we; + precedence=sw; + } f_sw = 0; + field { + sw=rw; + hw=w; we; + precedence=hw; + } f_hw = 0; + } r1 @ 0x0; + + reg { + default counter; + default sw=r; + default hw=na; + + field {} f_sw_count[3:0] = 0; + field {} f_hw_count[7:4] = 0; + } r1_events @ 0x4; + + r1_events.f_sw_count->incr = r1.f_sw; + r1_events.f_hw_count->incr = r1.f_hw; +}; diff --git a/test/test_precedence/tb_template.sv b/test/test_precedence/tb_template.sv new file mode 100644 index 0000000..b870bef --- /dev/null +++ b/test/test_precedence/tb_template.sv @@ -0,0 +1,26 @@ +{% extends "lib/tb_base.sv" %} + +{% block seq %} + {% sv_line_anchor %} + ##1; + cb.rst <= '0; + ##1; + + // Always write both fields from hardware + cb.hwif_in.r1.f_sw.next <= '0; + cb.hwif_in.r1.f_sw.we <= '1; + cb.hwif_in.r1.f_hw.next <= '0; + cb.hwif_in.r1.f_hw.we <= '1; + @cb; + @cb; + + cpuif.assert_read('h0, 'b00); + cpuif.assert_read('h4, 'h00); + + cpuif.write('h0, 'b11); + cpuif.write('h0, 'b11); + cpuif.write('h0, 'b11); + cpuif.assert_read('h0, 'h00); + cpuif.assert_read('h4, 'h03); + +{% endblock %} diff --git a/test/test_precedence/testcase.py b/test/test_precedence/testcase.py new file mode 100644 index 0000000..835b5ef --- /dev/null +++ b/test/test_precedence/testcase.py @@ -0,0 +1,5 @@ +from ..lib.sim_testcase import SimTestCase + +class Test(SimTestCase): + def test_dut(self): + self.run_test()