dbSta: make get_nets glob patterns find hierarchical nets#10939
dbSta: make get_nets glob patterns find hierarchical nets#10939oharboe wants to merge 2 commits into
Conversation
In a hierarchical netlist, get_nets with a wildcard returned nothing for nets inside hierarchical instances while the exact same name without a wildcard resolved, and get_pins/get_cells globs worked. dbSdcNetwork::findNetsMatching1 matched patterns against top-scope net names only. Hierarchical net names are module-local, so a pattern containing a divider can never match. Split the pattern at the last divider and search the scope of matching instances, mirroring findPinsMatching; keep the top-scope scan for flat names that contain dividers. dbNetwork::findInstNetsMatching ignored non-top instances, which also broke get_nets -hierarchical. Enumerate through netIterator; exact lookups stay on findNet. The get_nets1_hier test checks glob/exact agreement per scope, the -hierarchical variants, and a get_pins control on the hier1.v fixture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request adds support for finding nets inside hierarchical instances using glob and exact matching. Specifically, dbNetwork::findInstNetsMatching was refactored to support any instance rather than being restricted to the top instance, and dbSdcNetwork::findNetsMatching1 was updated to handle hierarchical paths in net patterns. A new integration test get_nets1_hier was also added to verify these changes. There are no review comments, so no feedback is provided.
|
@dsengupta0628 @maliberty Heads up! I don't know what I'm doing here, but the use-case is to reduce Claude token usage... This is a yak-shave by Claude because it spent a lot of time finding a signal and it thinks the root cause is this issue it fixed here. The change was so small, that I think someone who knows what is going on here will be able to see if Claude is right here. |
|
@codex review |
|
After this change the wildcard patterns seem to work correctly- one example on a hier1.v (top has b1, b2 of module block1; each has net u1out, r1q): So this code does fix an existing bug |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe1dc643fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| } else { | ||
| Net* net = findNet(instance, pattern->pattern()); |
There was a problem hiding this comment.
Avoid matching flat path names as leaf-local nets
When get_nets -hierarchical <name> walks into a leaf instance, this new exact-match path calls findNet(instance, ...), whose non-top fallback looks up pathName(instance) + "/" + net_name in the flat block. In flat designs, or hierarchical designs with escaped/top-scope net names containing dividers, a top-scope net like u1/foo can therefore be returned by get_nets -hierarchical foo while visiting leaf instance u1, even though there is no local net foo in that leaf scope. This changes hierarchical searches from matching local nets at each scope to also matching arbitrary flat names that happen to have an instance-name prefix; consider limiting this fallback to real hierarchical module instances or exact-matching through the instance net iterator.
Useful? React with 👍 / 👎.
|
Expanding on what Codex found: There is a bug in dbNetwork::findInstNetsMatching exact branch, dbNetwork.cc around L1293 now Suppose we have a flat netlist (Synthesis flattened a design); And net names carry the old path as literal text. One net is literally named u1/n5. There is a leaf instance named u1. No hierarchy - u1/n5 is just a flat string. If we do get_nets -hierarchical n5 get_nets -hierarchical */n5 and get_nets -hierarchical n5 both return u1/n5 This is buggy. Basically the two branches disagree:
Same command family, two different lookup mechanisms. Wildcard = scoped iterator (correct). Exact = flat string probe (leaky). Fixing exact to also go through netIterator makes them consistent and kills the false match. Minimal fix: collapse the branch - use the scoped iterator for both cases. PatternMatch::match already does exact compare when there are no wildcards, so the else is redundant and the source of the leak. |
| } | ||
| } | ||
| } else { | ||
| Net* net = findNet(instance, pattern->pattern()); |
There was a problem hiding this comment.
This is problematic. Please use the scoped iterator for both cases. PatternMatch::match already does exact compare when there are no wildcards, so the else is redundant and the source of the leak.
If we have a flat netlist, and net names carry the old path as literal text, with one net named u1/n5, then there is a leaf instance named u1. No hierarchy - u1/n5 is just a flat string.
If we do get_nets -hierarchical n5
( Meaning: find nets named n5 anywhere in hierarchy )
it will returns u1/n5 in new code. But u1 is a leaf with no scope net n5.
get_nets -hierarchical */n5 and get_nets -hierarchical n5 both return u1/n5
This is buggy.
So please collapse the two branches
findInstNetsMatching had two branches: glob went through netIterator (scoped, correct), exact went through findNet. For a non-top instance, findNet falls back to a flat block lookup of pathName(instance)+"/"+name, so get_nets -hierarchical <name> could return a top-scope net like u1/foo while visiting leaf instance u1, even though u1 has no local net foo. Collapse to the scoped iterator for both cases. PatternMatch::match does an exact compare when there are no wildcards, so the branch was redundant and the source of the false match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
In a hierarchical netlist, get_nets with a wildcard returned nothing
for nets inside hierarchical instances while the exact same name
without a wildcard resolved, and get_pins/get_cells globs worked.
dbSdcNetwork::findNetsMatching1 matched patterns against top-scope net
names only. Hierarchical net names are module-local, so a pattern
containing a divider can never match. Split the pattern at the last
divider and search the scope of matching instances, mirroring
findPinsMatching; keep the top-scope scan for flat names that contain
dividers.
dbNetwork::findInstNetsMatching ignored non-top instances, which also
broke get_nets -hierarchical. Enumerate through netIterator; exact
lookups stay on findNet.
The get_nets1_hier test checks glob/exact agreement per scope, the
-hierarchical variants, and a get_pins control on the hier1.v fixture.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Signed-off-by: Øyvind Harboe oyvind.harboe@zylin.com