[Do Not Merge Yet] 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

src/dbSta/src/dbNetwork.cc

+  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

src/dbSta/src/dbSta.i

  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

src/dbSta/src/dbNetwork.cc

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

…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! 👍"

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

dsengupta0628 · 2026-03-23T20:34:12Z

Secure-ci run with new set of changes: http://secure-ci:8080/view/My/job/SB/job/secure-fix_missing_para_dbmodnet/

github-actions · 2026-03-23T20:38:34Z

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

maliberty · 2026-03-23T23:14:14Z

Did you intend to update sta here? If so you have some issues in sta to address.

dsengupta0628 · 2026-03-23T23:23:50Z

Did you intend to update sta here? If so you have some issues in sta to address.

I did not. The fix strictly requires changes in dbSta. Let me force the sta/ to match whatever in master.

github-actions · 2026-03-23T23:36:02Z

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

jhkim-pii · 2026-03-24T00:25:20Z

src/dbSta/src/dbNetwork.cc

-          = related_dbnet->findModNetInHighestHier()) {
-        return dbToSta(highest_modnet);  // Found the highest modnet
-      }
+      return dbToSta(related_dbnet);


ReportPath::descriptionNet(const Pin *pin) uses the API.
Is this ok?

Net* dbNetwork::highestNetAbove that I modified is not called by ReportPath::descriptionNet(const Pin *pin). Rather it calls Network::highestNetAbove(Net *net) function. This highestNetAbove is defined virtual in the Network class

Okay I see what you mean. In OpenROAD binary, ReportPath::descriptionNet() dispatches to dbNetwork::highestNetAbove(). So the opposite is guaranteed: OpenROAD will always call dbNetwork::highestNetAbove(), not Network::highestNetAbove(). Hmm.. I don't know the answer yet- I don't see any regression failures, but I will get CC to check for this

Yes, dbNetwork::highestNetAbove() changes report_checks output (and any command using ReportPath::descriptionNet), but only for hierarchical designs with modNets — and it changes the net name displayed, not the timing values.

top --> u_core (instance of "core") --> u_alu (instance of "alu") --> adder gate drives wire "sum_out"

If the same physical signal has a different modNet name at each hierarchy level as shown above, after flattening, one flat dbNet is created. Its name depends on where/how flattening named it — suppose it's u_core/alu_result (a common convention: the net retains the name from an intermediate hierarchy level).
Then the visible difference would be:

The old behavior always showed the topmost hierarchical name — the name at the highest module boundary the signal reached. The new behavior shows the flat dbNet name, which is whatever name the net was assigned during flattening/synthesis.

What is NOT affected

Timing values (slack, delay, slew, capacitance) — completely unchanged

Path selection (which paths are reported) — unchanged

All non-net lines in the report — unchanged

The net line is only printed when report_net_ is enabled (the -nets option to report_checks), so default report_checks without -nets sees no difference at all

dsengupta0628 · 2026-03-24T00:46:36Z

QoR change in CI run. Will analyze the QoR changes in these 3 designs to ensure this is explainable change.

Number of designs: 63

------------------------------
       Failing designs
------------------------------
asap7 riscv32i (base)
  Flow reached last stage.
  Found 1 metrics failures.
      [ERROR] finish__timing__setup__tns fail test: -6385.82 >= -4450

asap7 swerv_wrapper (base)
  Flow reached last stage.
  Found 2 metrics failures.
      [ERROR] finish__timing__setup__ws fail test: -625.186 >= -276
      [ERROR] finish__timing__setup__tns fail test: -251445 >= -12900

sky130hd microwatt (base)
  Flow reached last stage.
  Found 3 metrics failures.
      [ERROR] finish__timing__setup__ws fail test: -1.50039 >= -1.38
      [ERROR] finish__timing__setup__tns fail test: -237.787 >= -61.5
      [ERROR] finish__timing__hold__tns fail test: -22.9742 >= -22.1

github-actions · 2026-03-25T04:00:26Z

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

dsengupta0628 · 2026-03-25T04:02:18Z

QoR change in CI run. Will analyze the QoR changes in these 3 designs to ensure this is explainable change.

Number of designs: 63

------------------------------
       Failing designs
------------------------------
asap7 riscv32i (base)
  Flow reached last stage.
  Found 1 metrics failures.
      [ERROR] finish__timing__setup__tns fail test: -6385.82 >= -4450

asap7 swerv_wrapper (base)
  Flow reached last stage.
  Found 2 metrics failures.
      [ERROR] finish__timing__setup__ws fail test: -625.186 >= -276
      [ERROR] finish__timing__setup__tns fail test: -251445 >= -12900

sky130hd microwatt (base)
  Flow reached last stage.
  Found 3 metrics failures.
      [ERROR] finish__timing__setup__ws fail test: -1.50039 >= -1.38
      [ERROR] finish__timing__setup__tns fail test: -237.787 >= -61.5
      [ERROR] finish__timing__hold__tns fail test: -22.9742 >= -22.1

I checked one design asap/riscv32i with a binary without my changes but Martin's check_para_annotation utility and see that this indeed had many missing para annotations. So now some of these testcases will indeed change QoR but hopefully this is a correct change and we can absorb this.

dsengupta0628 · 2026-03-25T04:17:19Z

ORFS PR for metrics update: The-OpenROAD-Project/OpenROAD-flow-scripts#4050

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

github-actions · 2026-03-30T14:37:45Z

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

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 suggested 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

update latest src/sta

51a1bb5

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

dsengupta0628 requested a review from maliberty March 23, 2026 20:45

Force-align STA submodule to master (d213f0f)

62d242f

jhkim-pii reviewed Mar 24, 2026

View reviewed changes

Merge branch 'master' into missing_para_dbmodnet

ea8cf58

openroad-ci mentioned this pull request Mar 25, 2026

Secure fix missing para dbmodnet The-OpenROAD-Project/OpenROAD-flow-scripts#4050

Open

dsengupta0628 added 2 commits March 30, 2026 14:26

Merge branch 'master' into missing_para_dbmodnet

8603606

updated with latest sta and OR master

1623227

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

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!

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!

dsengupta0628 commented Mar 23, 2026

Uh oh!

github-actions bot commented Mar 23, 2026

Uh oh!

maliberty commented Mar 23, 2026

Uh oh!

dsengupta0628 commented Mar 23, 2026

Uh oh!

github-actions bot commented Mar 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsengupta0628 commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

dsengupta0628 commented Mar 25, 2026

Uh oh!

dsengupta0628 commented Mar 25, 2026

Uh oh!

github-actions bot commented Mar 30, 2026

dsengupta0628 commented Mar 13, 2026 •

edited

Loading

dsengupta0628 Mar 24, 2026 •

edited

Loading