Missing para dbmodnet by openroad-ci · Pull Request #9745 · The-OpenROAD-Project/OpenROAD

openroad-ci · 2026-03-12T21:51:40Z

OpenROAD's ODB maintains two net representations simultaneously for hierarchical designs:

dbNet (flat): the physical wire — one per connected signal on the chip
dbModNet (hierarchical): a logical net scoped to a module level — multiple can exist for the same physical signal at different hierarchy levels

A flat dbITerm (leaf cell pin) can be connected to both. dbNetwork::net(pin) returns the modnet wrapper first

The bug (before fix)

highestConnectedNet is called by findParasiticNet to get the canonical lookup key for the parasitic map:

findParasiticNet(driver_pin)
→ network_->net(driver_pin) # returns modnet STA wrapper (priority)
→ network_->highestConnectedNet() # OLD: trivially returns net unchanged
→ parasitic_network_map_.find(modnet_key) # MISS — SPEF stored under flat net key
→ nullptr

SPEF is a flat format — when read, parasitics are always stored keyed by the flat dbNet STA wrapper. But the lookup was being done with the modnet STA wrapper → cache miss → parasitic = nullptr → LumpedCap fallback → And this was hitting the bug in LumpedCapDelayCalc::makeResult (tracked separately with James Cherry)

With the fix, findRelatedNet() walks the modnet's connected flat dbITerm objects and returns the first iterm->getNet() it finds — which is the flat dbNet that the SPEF parasitic is keyed under.

gemini-code-assist

Code Review

This pull request addresses a bug in parasitic extraction for hierarchical designs by ensuring highestConnectedNet correctly resolves a dbModNet to its corresponding flat dbNet. The logic in dbNetwork.cc is sound and directly fixes the described issue. The addition of the parasitics_annotated helper function in dbSta.i is a useful addition for testing and scripting. The minor suggestion in dbNetwork.cc to improve code clarity and conciseness remains valid.

gemini-code-assist · 2026-03-12T21:54:43Z

+  dbNet* db_net = nullptr;
+  odb::dbModNet* db_modnet = nullptr;
+  staToDb(net, db_net, db_modnet);
+  if (db_modnet) {
+    dbNet* flat_net = db_modnet->findRelatedNet();
+    if (flat_net) {
+      return dbToSta(flat_net);
+    }
+  }
  return net;


This implementation is correct. It can be slightly refactored for conciseness and to clarify intent regarding an unused variable. The db_net variable is required for the staToDb call but is not used thereafter. Renaming it to db_net_unused makes this explicit. Additionally, the nested if can be simplified using a C++17 if-statement with an initializer, which also scopes the flat_net variable more tightly.

dbNet* db_net_unused = nullptr; odb::dbModNet* db_modnet = nullptr; staToDb(net, db_net_unused, db_modnet); if (db_modnet) { if (dbNet* flat_net = db_modnet->findRelatedNet()) { return dbToSta(flat_net); } } return net;

Net* dbNetwork::findFlatNet(const Net* net) does the same thing.
You can simply call it.

Thank you. I think I did this because of the difference in fallback logic. But there is a simple change that can fix that and use findFlatNet instead, just uploaded it now.

github-actions · 2026-03-12T21:56:46Z

clang-tidy review says "All clean, LGTM! 👍"

maliberty

Looks fine but I think worth a secure-CI run as it may affect parasiti

maliberty · 2026-03-12T21:56:20Z

@@ -228,6 +228,11 @@ void check_axioms_cmd()
  sta->checkSanity();
 }

+bool parasitics_annotated(Pin *pin, Scene *scene) {


I dropped this commit- it was from Martin

dsengupta0628 · 2026-03-12T22:00:01Z

Looks fine but I think worth a secure-CI run as it may affect parasiti

Secure-CI ongoing https://jenkins.openroad.tools/job/OpenROAD-flow-scripts-Private/job/secure-fix_missing_para_dbmodnet/

jhkim-pii · 2026-03-13T00:37:56Z

The objective of Network::highestConnectedNet(Net* net) is to retrieve the hierarchical net in the highest module.
But the this dbNetwork::highestConnectedNet(Net* net) finds the corresponding flat net for the given hier net, which is a different behavior.

Fortunately, there is only one caller Parasitics::findParasiticNet(const Pin *pin). So the current fix works.
But if Network::highestConnectedNet(Net* net) is used by another caller for a different purpose later, this fix can make a trouble.

I don't have a better idea about how to solve this issue.
But let's leave a comment to describe this issue.

e.g.,

// Caution: // - `Network::highestConnectedNet(Net *net)` retrieves the highest hierarchical net connected to the given net. // - But `dbNetwork::highestConnectedNet(Net* net)` retrieves the corresponding flat net for the given net. // It behaves differently to cope with the issue #9724. // - This redefinition may cause another issue later when `Network::highestConnectedNet(Net *net)` is used elsewhere.

I don't have a better idea about how to solve this issue.

To me the clean solution is to only ever expose hierarchical nets to STA since that seems to be what it's expecting and that's how ConcreteNetwork implements Network

Unfortunately that isn't possible with the way hierarchy was implemented in odb. For a net that is local to a single module we only create the flat net and skip the hierarchical one (since it would be equivalent as the net doesn't span a boundary). That complicates the API but does save memory on a flat design. The tradeoff could be revisited.

@maliberty in that case shouldn't we expose dbNet to STA only for those nets local to a module, and use dbModNet otherwise? That would match ConcreteNetwork as far as STA could tell

That is what we do.

This is what my

To me the clean solution is to only ever expose hierarchical nets...

comment was referring to. We don't consistently use dbModNet for nets which have them, which is a source of perpetual trouble.

I consider that a bug but the model remains as I stated it. I agree it would be simpler if we always allocated the modnet even if it was a duplicate of the dbNet. The downside is for a flat design it would double the net memory usage.

I'm not advocating for allocating dbmodnet in the trivial case but pointing out the bug. I understand the model.

Maybe I didn't understand "in that case shouldn't we expose dbNet to STA only for those nets local to a module, and use dbModNet otherwise?" - were you asking about the model or pointing out a bug?

It's because of the context of the thread (what should highestConnectedNet do) that I thought the bug is implied

…tails Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

github-actions · 2026-03-13T16:22:18Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2026-03-13T16:32:26Z

clang-tidy review says "All clean, LGTM! 👍"

dsengupta0628 · 2026-03-13T16:34:09Z

Looks fine but I think worth a secure-CI run as it may affect parasiti

Secure-CI ongoing https://jenkins.openroad.tools/job/OpenROAD-flow-scripts-Private/job/secure-fix_missing_para_dbmodnet/

I see some concerning things in this CI. There is a crash in a design (asap/swerv_wrapper), plus one design still triggered Martin's missing parasitics check (ihp-sg13g2/i2c-gpio-extender). So I will need to debug this a bit more before this PR can be merged

dsengupta0628 · 2026-03-17T20:45:15Z

There were secure-CI failures with this change- please do not merge this before I review those. Thanks

maliberty · 2026-03-17T20:45:16Z

Does this need a metrics update?

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 · 2026-03-19T21:32:13Z

There is a crash in asap7/werv_wrapper with current change. The crash is a threshold_times_ out-of-bounds access in PrimaDelayCalc::measureThresholds, triggered because node_count_ = 0 for certain nets in swerv_wrapper after this fix enables PRIMA to run on nets it never touched before.

This happens because the highestConnectedNet change while correct to solve the null return for findParasiticNetwork, exposes a secondary bug: the isExternal flag in parasitic pin nodes was set incorrectly during SPEF loading for hierarchical nets. Now every pin node in the parasitic network for a hierarchically-connected net is flagged external.

Before my fix

findParasiticNetwork(pin) → findParasiticNet(pin) → highestConnectedNet(net) → returned mod net → parasitic_network_map_.find(mod_net) → null → findParasitic() returns null → gateDelays() hits failed = true → falls back to tableDcalcResults(). PRIMA never ran. No crash.

I am exploring what best additional fix needed to take care of this.

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 · 2026-03-20T00:57:00Z

New CI run ongoing: https://jenkins.openroad.tools/job/OpenROAD-flow-scripts-Private/job/secure-fix_missing_para_dbmodnet/3/stages/

github-actions · 2026-03-20T00:59:40Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2026-05-15T21:12:20Z

clang-tidy review says "All clean, LGTM! 👍"

dsengupta0628

will likely need ORFS rebasing. check

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 · 2026-05-18T19:40:52Z

@codex review

github-actions · 2026-05-18T19:45:11Z

clang-tidy review says "All clean, LGTM! 👍"

chatgpt-codex-connector · 2026-05-18T19:48:19Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ 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".

dsengupta0628 · 2026-05-18T21:40:15Z

ORFS PR: The-OpenROAD-Project/OpenROAD-flow-scripts#4050
Latest secure ci ongoing: http://secure-ci:8080/view/My/job/SB/job/secure-fix_missing_para_dbmodnet/

dsengupta0628

Codex did not find any issue

github-actions · 2026-05-19T17:14:28Z

clang-tidy review says "All clean, LGTM! 👍"

dsengupta0628

I am the author so I approve my change (though we may need GHA change to not require my review on my authored PRs)

maliberty · 2026-05-20T06:30:44Z

any metrics update required?

dsengupta0628 · 2026-05-20T10:23:37Z

any metrics update required?

Yes. Already added the metrics PR in the excel doc. ORFS PR: The-OpenROAD-Project/OpenROAD-flow-scripts#4050

openroad-ci assigned dsengupta0628 Mar 12, 2026

dsengupta0628 requested review from jhkim-pii, maliberty and povik March 12, 2026 21:51

gemini-code-assist Bot reviewed Mar 12, 2026

View reviewed changes

maliberty reviewed Mar 12, 2026

View reviewed changes

jhkim-pii reviewed Mar 13, 2026

View reviewed changes

jhkim-pii requested changes Mar 13, 2026

View reviewed changes

dsengupta0628 added 3 commits March 13, 2026 16:21

Fix for issue 9274 Lost parasitic annotations because of dbNetwork de…

39ac5ac

…tails Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

simplfy logic per feedback

35f274e

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Added comments for caution

09730f1

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

openroad-ci force-pushed the missing_para_dbmodnet branch from fb1b762 to 09730f1 Compare March 13, 2026 16:27

maliberty approved these changes Mar 17, 2026

View reviewed changes

maliberty changed the title ~~Missing para dbmodnet~~ [Do Not Merge Yet] Missing para dbmodnet Mar 17, 2026

dsengupta0628 added 2 commits March 19, 2026 14:09

Merge branch 'master' into missing_para_dbmodnet

0489b20

updated sta to latest on 3/19

46be33b

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 added 3 commits March 20, 2026 00:15

Merge remote-tracking branch 'origin/master' into missing_para_dbmodnet

b19079f

isExternal issue fix to resolve crash in PRIMA delay calc

6832007

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

reformatted clang

73558ab

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Merge branch 'master' into missing_para_dbmodnet

b614eb5

Merge branch 'master' into missing_para_dbmodnet

1ca316c

openroad-ci requested a review from a team as a code owner May 15, 2026 21:07

github-actions Bot added the size/S label May 15, 2026

dsengupta0628 reviewed May 15, 2026

View reviewed changes

dsengupta0628 added 4 commits May 18, 2026 14:16

enhanced cpp test and one tcl test to explicitly show the fix

757e8a0

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Merge branch 'master' into missing_para_dbmodnet

d71d23e

regolden regression

923a5ee

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

tclfmt

32458ec

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 requested review from a team May 18, 2026 19:39

github-actions Bot added size/M and removed size/S labels May 18, 2026

dsengupta0628 requested a review from jhkim-pii May 18, 2026 19:46

dsengupta0628 changed the title ~~[Do Not Merge Yet] Missing para dbmodnet~~ Missing para dbmodnet May 18, 2026

dsengupta0628 reviewed May 19, 2026

View reviewed changes

Merge branch 'master' into missing_para_dbmodnet

bc799c6

dsengupta0628 requested a review from a team May 19, 2026 17:09

maliberty removed request for a team May 19, 2026 22:10

dsengupta0628 approved these changes May 20, 2026

View reviewed changes

jhkim-pii approved these changes May 20, 2026

View reviewed changes

Conversation

openroad-ci commented Mar 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 12, 2026

Uh oh!

maliberty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsengupta0628 commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 13, 2026

Uh oh!

github-actions Bot commented Mar 13, 2026

Uh oh!

dsengupta0628 commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsengupta0628 commented Mar 17, 2026

Uh oh!

maliberty commented Mar 17, 2026

Uh oh!

dsengupta0628 commented Mar 19, 2026

Before my fix

Uh oh!

dsengupta0628 commented Mar 20, 2026

Uh oh!

github-actions Bot commented Mar 20, 2026

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

dsengupta0628 left a comment

Choose a reason for hiding this comment

Uh oh!

dsengupta0628 commented May 18, 2026

Uh oh!

github-actions Bot commented May 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 18, 2026

Uh oh!

dsengupta0628 commented May 18, 2026

Uh oh!

dsengupta0628 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 19, 2026

dsengupta0628 commented Mar 13, 2026 •

edited

Loading