From 3e691cb5fb06fd74dd0fcae522edae64bb334413 Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Sun, 14 May 2023 22:46:16 -0700 Subject: [PATCH] Fix bug where small designs with 3 or less sw readable addresses and readback retiming enabled generate incorrect output. --- src/peakrdl_regblock/exporter.py | 28 ++++++++++++++----- src/peakrdl_regblock/module_tmpl.sv | 2 +- src/peakrdl_regblock/readback/__init__.py | 6 ++-- .../readback/templates/readback.sv | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/peakrdl_regblock/exporter.py b/src/peakrdl_regblock/exporter.py index b19d283..c92f7f0 100644 --- a/src/peakrdl_regblock/exporter.py +++ b/src/peakrdl_regblock/exporter.py @@ -157,6 +157,12 @@ class RegblockExporter: # Validate that there are no unsupported constructs DesignValidator(self).do_validate() + # Compute readback implementation early. + # Readback has the capability to disable retiming if the fanin is tiny. + # This affects the rest of the design's implementation, and must be known + # before any other templates are rendered + readback_implementation = self.readback.get_implementation() + # Build Jinja template context context = { "cpuif": self.cpuif, @@ -167,7 +173,7 @@ class RegblockExporter: "default_resetsignal_name": self.dereferencer.default_resetsignal_name, "address_decode": self.address_decode, "field_logic": self.field_logic, - "readback": self.readback, + "readback_implementation": readback_implementation, "ext_write_acks": ext_write_acks, "ext_read_acks": ext_read_acks, "get_always_ff_event": self.dereferencer.get_always_ff_event, @@ -224,8 +230,6 @@ class DesignState: #------------------------ # Info about the design #------------------------ - self.min_read_latency = 0 - self.min_write_latency = 0 self.cpuif_data_width = 0 # Collections of signals that were actually referenced by the design @@ -243,13 +247,23 @@ class DesignState: self.user_enums = [] # type: List[Type[UserEnum]] #------------------------ - if self.retime_read_fanin: - self.min_read_latency += 1 - if self.retime_read_response: - self.min_read_latency += 1 self.addr_width = self.top_node.size.bit_length() if user_addr_width is not None: if user_addr_width < self.addr_width: msg.fatal(f"User-specified address width shall be greater than or equal to {self.addr_width}.") self.addr_width = user_addr_width + + @property + def min_read_latency(self) -> int: + n = 0 + if self.retime_read_fanin: + n += 1 + if self.retime_read_response: + n += 1 + return n + + @property + def min_write_latency(self) -> int: + n = 0 + return n diff --git a/src/peakrdl_regblock/module_tmpl.sv b/src/peakrdl_regblock/module_tmpl.sv index c7e4527..0d91b33 100644 --- a/src/peakrdl_regblock/module_tmpl.sv +++ b/src/peakrdl_regblock/module_tmpl.sv @@ -226,7 +226,7 @@ module {{ds.module_name}} ( logic readback_err; logic readback_done; logic [{{cpuif.data_width-1}}:0] readback_data; - {{readback.get_implementation()|indent}} + {{readback_implementation|indent}} {% if ds.retime_read_response %} always_ff {{get_always_ff_event(cpuif.reset)}} begin if({{get_resetsignal(cpuif.reset)}}) begin diff --git a/src/peakrdl_regblock/readback/__init__.py b/src/peakrdl_regblock/readback/__init__.py index da441b1..dafb1e0 100644 --- a/src/peakrdl_regblock/readback/__init__.py +++ b/src/peakrdl_regblock/readback/__init__.py @@ -10,7 +10,6 @@ if TYPE_CHECKING: class Readback: def __init__(self, exp:'RegblockExporter'): self.exp = exp - self.do_fanin_stage = self.ds.retime_read_fanin @property def ds(self) -> 'DesignState': @@ -28,7 +27,7 @@ class Readback: # Enabling the fanin stage doesnt make sense if readback fanin is # small. This also avoids pesky corner cases if array_size < 4: - self.do_fanin_stage = False + self.ds.retime_read_fanin = False context = { "array_assignments" : array_assignments, @@ -36,11 +35,10 @@ class Readback: 'get_always_ff_event': self.exp.dereferencer.get_always_ff_event, 'get_resetsignal': self.exp.dereferencer.get_resetsignal, "cpuif": self.exp.cpuif, - "do_fanin_stage": self.do_fanin_stage, "ds": self.ds, } - if self.do_fanin_stage: + if self.ds.retime_read_fanin: # If adding a fanin pipeline stage, goal is to try to # split the fanin path in the middle so that fanin into the stage # and the following are roughly balanced. diff --git a/src/peakrdl_regblock/readback/templates/readback.sv b/src/peakrdl_regblock/readback/templates/readback.sv index b98d3e6..e44c9ed 100644 --- a/src/peakrdl_regblock/readback/templates/readback.sv +++ b/src/peakrdl_regblock/readback/templates/readback.sv @@ -4,7 +4,7 @@ logic [{{cpuif.data_width-1}}:0] readback_array[{{array_size}}]; {{array_assignments}} -{%- if do_fanin_stage %} +{%- if ds.retime_read_fanin %} // fanin stage logic [{{cpuif.data_width-1}}:0] readback_array_c[{{fanin_array_size}}];