Repository navigation
Conversation
slang's --keep-hierarchy elaborates one module per instance and names it `<definition>$<instance path>`. The detector named a memory after its module, so a memory module instanced twice produced two entries, two liberty views of one array, and two uniquified names in blackboxes.txt. read_slang's --blackboxed-module matches the definition's name only and ignores a name it does not know, so nothing was blackboxed: every array was elaborated, flattened into its parent, and synthesis stopped at SYNTH_MEMORY_MAX_BITS. asap7/tage_update (The-OpenROAD-Project#4595) instances each of its SRAM definitions 8 to 16 times and hits this. The detector now cuts a module name at the first `$` when an identifier precedes it (yosys's own `$paramod\...` names have nothing before it and stay whole) and collapses the copies into one entry. Copies that disagree in shape or pins are an error: one liberty view per definition cannot serve a definition parameterised on its memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![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 adds support for handling slang's --keep-hierarchy module naming convention by extracting the base definition name and deduplicating elaborated memory instances. It ensures that multiple copies of a definition agree on shape and pins, raising an error otherwise. The feedback suggests simplifying the definition_name check to correctly handle escaped Verilog identifiers starting with digits, adding a test case for this scenario, and refactoring the pin comparison in dedupe to compare Pin objects directly for improved robustness.
Review: cutting at the first `$` breaks a definition whose own name has a `$` (legal in a Verilog identifier): read_slang names an instance of `my$ram` under top `top2` `my$ram$top2.u2`, and the first-`$` cut made it `my`, a module that does not exist. The instance path starts at the top module, so the name is now cut where `$<top>.` begins; gen_memories takes the top as --top, which the Makefile passes as DESIGN_NAME. An escaped definition starting with a digit (`\1d_ram`, which read_slang blackboxes as `1d_ram`) is cut the same way. Copies of one definition are compared on their whole pins, port and function included, not on name, direction and width only. 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), 0 without a comparable baseline. |
maliberty
approved these changes
Oct 11, 2026
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
3 participants
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.
slang's
--keep-hierarchyelaborates one module per instance and names it<definition>$<instance path>. The detector named a memory after its module, so a memory module instanced twice produced two entries, two liberty views of one array, and two uniquified names inblackboxes.txt.read_slang's--blackboxed-modulematches the definition's name only and ignores a name it does not know, so nothing was blackboxed: every array was elaborated, flattened into its parent, and synthesis stopped atSYNTH_MEMORY_MAX_BITS.The detector now cuts a module name at the first
$when an identifier precedes it (yosys's own$paramod\...names have nothing before it and stay whole) and collapses the copies into one entry. Copies that disagree in shape or pins are an error: one liberty view per definition cannot serve a definition parameterised on its memory.Found with asap7/tage_update (#4595), which instances each of its SRAM definitions 8 to 16 times; with this fix
blackboxes.txtholdsmbtb_entry_sram,tage_entry_sramandtage_useful_sram, and synthesis completes.🤖 Generated with Claude Code