Repository navigation
Conversation
This was referenced Nov 3, 2026
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to drop parameters from blackboxed parameterized memory modules, preventing OpenROAD's Verilog reader from failing on them. It adds a new Tcl script to unset these parameters, integrates it into the synthesis preamble, and includes a Python test suite to verify the behavior. The review feedback suggests optimizing the parameter-dropping script by collecting and deduplicating parameter names before calling setparam, which avoids redundant operations and improves performance from O(N^2) to O(N).
oharboe
added a commit
to oharboe/OpenROAD-flow-scripts
that referenced
this pull request
Nov 3, 2026
CoreMiniAxi.sv is regenerated from coralnpu b50b85a9 by src/coralnpu/generate, and the design is configured for it: - The five TCM banks (ITCM one, DTCM four) are coralnpu's Sram, 512 x 128 with one read-write port and a 16-lane byte mask. sram.memories names its pins (ADDITIONAL_MEMORIES) and AUTO_MEMORIES replaces it with a generated macro, write mask and all. The previous build swapped in USE_ASAP7 fakerams, which have no write mask, so a masked write wrote all 16 bytes. - Regfile and FRegfile keep their words as separate registers, with no array for yosys to infer. surgery.py (SYNTH_VERILOG_SURGERY) moves them into Regfile_rf and FRegfile_rf, which AUTO_MEMORIES_REGFILES has generate_ram build as standard cells, every word with its asynchronous reset: Regfile 32 x 32 with x0 reading zero, nine reads and six writes; FRegfile 32 x 32, four reads and two writes. - The clock is 1,900 ps in the platform's picoseconds (it was 10.00), with the platform's set_max_delay IO budgets; the asynchronous reset is not timed. The kept hierarchy is coralnpu's units by name, with OPENROAD_HIERARCHICAL=1. - The divider's 128 x 83 SRT table is a ROM yosys maps to logic; a ROM is not counted against SYNTH_MEMORY_MAX_BITS. surgery.py recognises each register file token for token and stops the build on anything else. Each was simulated before and after the surgery, and generate_ram's netlists against the behavioural modules the surgery writes, on the same random inputs, every output compared every cycle, with a negative control for each that is caught. Needs AUTO_MEMORIES_REGFILES (The-OpenROAD-Project#4781), the supply pins of dont_touch instances connected (The-OpenROAD-Project#4782) and a parameterized module as a macro (The-OpenROAD-Project#4783). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
AUTO_MEMORIES blackboxes a parameterized memory module (coralnpu's Sram #(NUM_ENTRIES, GLOBAL_BASE_ADDR)), and the black box keeps its instances' parameters: yosys writes `Sram #(.NUM_ENTRIES(...)) ...` into 1_2_yosys.v, and OpenROAD's Verilog reader stops on it (STA-0171, syntax error). The macro is the module at the widths detection sized it: the parameters are dropped from its instances right after the sources are read. flow/test/test_auto_memories_parameters.py checks that the black box keeps the parameters without the change, and that the instance is written without them, its ports unchanged, with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
oharboe
force-pushed
the
auto-memories-macro-parameters
branch
from
November 3, 2026 08:14
ed8a1ac to
9b7e836
Compare
oharboe
marked this pull request as ready for review
November 3, 2026 08:28
oharboe
added a commit
to oharboe/OpenROAD-flow-scripts
that referenced
this pull request
Nov 3, 2026
CoreMiniAxi.sv is regenerated from coralnpu b50b85a9 by src/coralnpu/generate, and the design is configured for it: - The five TCM banks (ITCM one, DTCM four) are coralnpu's Sram, 512 x 128 with one read-write port and a 16-lane byte mask. sram.memories names its pins (ADDITIONAL_MEMORIES) and AUTO_MEMORIES replaces it with a generated macro, write mask and all. The previous build swapped in USE_ASAP7 fakerams, which have no write mask, so a masked write wrote all 16 bytes. - Regfile and FRegfile keep their words as separate registers, with no array for yosys to infer. surgery.py (SYNTH_VERILOG_SURGERY) moves them into Regfile_rf and FRegfile_rf, which AUTO_MEMORIES_REGFILES has generate_ram build as standard cells, every word with its asynchronous reset: Regfile 32 x 32 with x0 reading zero, nine reads and six writes; FRegfile 32 x 32, four reads and two writes. - The clock is 1,900 ps in the platform's picoseconds (it was 10.00), with the platform's set_max_delay IO budgets; the asynchronous reset is not timed. The kept hierarchy is coralnpu's units by name, with OPENROAD_HIERARCHICAL=1. - The divider's 128 x 83 SRT table is a ROM yosys maps to logic; a ROM is not counted against SYNTH_MEMORY_MAX_BITS. surgery.py recognises each register file token for token and stops the build on anything else. Each was simulated before and after the surgery, and generate_ram's netlists against the behavioural modules the surgery writes, on the same random inputs, every output compared every cycle, with a negative control for each that is caught. Needs AUTO_MEMORIES_REGFILES (The-OpenROAD-Project#4781), the supply pins of dont_touch instances connected (The-OpenROAD-Project#4782) and a parameterized module as a macro (The-OpenROAD-Project#4783). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Member
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 0 with regression(s), 1 without a comparable baseline. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before, AUTO_MEMORIES left a parameterized memory module's parameters on its black-boxed instances: yosys wrote
Sram #(.NUM_ENTRIES(...)) u0into1_2_yosys.v, and OpenROAD's Verilog reader stopped on it (STA-0171).Now the parameters are dropped from the instances of every AUTO_MEMORIES black box right after the sources are read. The macro is the module at the widths detection sized it.
This wires up more of generate_ram's experimental features in the flow. It is one of three changes needed before asap7/coralnpu (#4653) can go in, whose SRAMs are instances of
Sram #(NUM_ENTRIES, GLOBAL_BASE_ADDR), with #4781 and #4782. The three are independent of each other. #4781 drops the parameters of inlined register files the same way; whichever of the two merges second shares the proc.Tests:
flow/test/test_auto_memories_parameters.py(skipped without yosys withread_slang) checks that the black box keeps the parameters without the change, and that with it the instance is written without them, its ports unchanged. Withsetparam -unsetremoved, the second test fails.🤖 Generated with Claude Code