Repository navigation
Conversation
global_connect skips dont_touch instances (ODB-0383, deliberate in odb), so an instance made dont_touch before the PDN step keeps its VDD and VSS pins on no net through final. A register file built by generate_ram and dissolved into its parent after macro placement is that case: its core is FIRM and dont_touch by the PDN step. 896 of riscv32i's 7,089 instances with its register file generated, 406,272 cells of XiangShan's XSTile, were left unpowered. The M1 rails touch them, so timing and routing were unaffected; the netlist, LVS and power analysis saw them unpowered. The PDN step lifts dont_touch from every dont_touch instance with a supply pin on no net while the platform's PDN script makes its global connections, and restores it after: one connection, no ODB-0383 warning per instance. A supply pin already on a net is left alone. flow/test/test_pdn_dont_touch_supplies.py checks that global_connect still skips a dont_touch instance, that only the dont_touch instances with a supply pin on no net are named, and that they are connected and dont_touch again afterwards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
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 temporarily lift the dont_touch property from instances with unsupplied supply pins during the PDN step, ensuring that global_connect can connect them properly. This is implemented via a new dont_touch_unsupplied helper in util.tcl and integrated into pdn.tcl, alongside a new Python test suite to verify the behavior. The single review comment was removed because it violates the repository style guide by suggesting a silent fallback (returning an empty list) when no design is loaded, rather than allowing a loud failure. Therefore, there is no further feedback to provide.
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>
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. |
Contributor
|
@oharboe this feel hacky to me. It seems like the right approach is to create the global connect rules after loading in the netlist and then you can apply the dont_touch after that without having to worry about this. |
Collaborator
Author
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, an instance marked dont_touch before the PDN step kept its VDD and VSS pins on no net through final, because
global_connectskips dont_touch instances (ODB-0383).Now the PDN step connects them: dont_touch is lifted from those instances while the platform's PDN script makes its global connections, and restored after.
The M1 rails touch these cells, so timing and routing were unaffected; the netlist, LVS and power analysis saw them unpowered.
This wires up more of generate_ram's experimental features in the flow: a register file dissolved into its parent after macro placement (#4781) has its core FIRM and dont_touch by the PDN step. It is one of three changes needed before asap7/coralnpu (#4653) can go in, with #4781 and #4783. The three are independent of each other; this one is a fix for any flow that marks instances dont_touch before PDN.
A supply pin already on a net is left alone. One info message (FLW-0018) gives the count, in place of an ODB-0383 warning per instance.
Tests:
flow/test/test_pdn_dont_touch_supplies.py(skipped withoutopenroad) checks thatglobal_connectstill skips a dont_touch instance, that only dont_touch instances with a supply pin on no net are named, and that they are connected and dont_touch again afterwards. With the proc returning nothing, two of the three tests fail.🤖 Generated with Claude Code