Skip to content

[Do Not Merge Yet] Missing para dbmodnet #9745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
openroad-ci wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[Do Not Merge Yet] Missing para dbmodnet #9745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
39ac5ac
Fix for issue 9274 Lost parasitic annotations because of dbNetwork de…
dsengupta0628 Mar 12, 2026
35f274e
simplfy logic per feedback
dsengupta0628 Mar 13, 2026
09730f1
Added comments for caution
dsengupta0628 Mar 13, 2026
0489b20
Merge branch 'master' into missing_para_dbmodnet
dsengupta0628 Mar 19, 2026
46be33b
updated sta to latest on 3/19
dsengupta0628 Mar 19, 2026
b19079f
Merge remote-tracking branch 'origin/master' into missing_para_dbmodnet
dsengupta0628 Mar 20, 2026
6832007
isExternal issue fix to resolve crash in PRIMA delay calc
dsengupta0628 Mar 20, 2026
73558ab
reformatted clang
dsengupta0628 Mar 20, 2026
b614eb5
Merge branch 'master' into missing_para_dbmodnet
dsengupta0628 Mar 23, 2026
51a1bb5
update latest src/sta
dsengupta0628 Mar 23, 2026
62d242f
Force-align STA submodule to master (d213f0f)
dsengupta0628 Mar 23, 2026
ea8cf58
Merge branch 'master' into missing_para_dbmodnet
dsengupta0628 Mar 25, 2026
8603606
Merge branch 'master' into missing_para_dbmodnet
dsengupta0628 Mar 30, 2026
1623227
updated with latest sta and OR master
dsengupta0628 Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/dbSta/include/db_sta/dbNetwork.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,10 @@ class dbNetwork : public ConcreteNetwork

// Return the highest net above the given net.
// - If the net is a flat net, return it.
// - If the net is a hier net, return the modnet in the highest hierarchy.
// - If the net is a hier net (dbModNet), return the associated flat dbNet.
// This ensures parasitic externality checks in ensureParasiticNode work
// correctly: net_ is always a flat net, so the comparison net != net_
// must also operate on flat nets.
Net* highestNetAbove(Net* net) const override;

////////////////////////////////////////////////////////////////
Expand Down
23 changes: 18 additions & 5 deletions src/dbSta/src/dbNetwork.cc
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@jhkim-pii jhkim-pii Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@povik povik Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

@maliberty maliberty Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@povik povik Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Original file line number Diff line number Diff line change
Expand Up @@ -2016,9 +2016,18 @@ void dbNetwork::visitConnectedPins(const Net* net,
}
}

// 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.
const Net* dbNetwork::highestConnectedNet(Net* net) const
{
return net;
const Net* flat = findFlatNet(net);
return flat ? flat : net;
}

////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -5198,11 +5207,15 @@ Net* dbNetwork::highestNetAbove(Net* net) const
}

if (modnet) {
// Return the flat net associated with this mod net.
// Parasitic externality checks in
// ConcreteParasiticNetwork::ensureParasiticNode compare against net_ which
// is always a flat net (set via makeParasiticNetwork). Returning the
// highest mod net causes all pin nodes on hierarchically-connected nets to
// compare unequal to net_ and be incorrectly marked as external, making
// node_count_ = 0 and crashing PRIMA in measureThresholds.
if (dbNet* related_dbnet = modnet->findRelatedNet()) {
if (odb::dbModNet* highest_modnet
= related_dbnet->findModNetInHighestHier()) {
return dbToSta(highest_modnet); // Found the highest modnet
}
return dbToSta(related_dbnet);
Copy link
Copy Markdown
Contributor

@jhkim-pii jhkim-pii Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Image

Copy link
Copy Markdown
Contributor

@dsengupta0628 dsengupta0628 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@dsengupta0628 dsengupta0628 Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@dsengupta0628 dsengupta0628 Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Image

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:

Image

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

}
}

Expand Down
4 changes: 3 additions & 1 deletion src/dbSta/test/cpp/TestDbSta.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ TEST_F(TestDbSta, TestHierarchyConnectivity)
ASSERT_NE(modnet_out2, nullptr);
Net* sta_modnet_out2 = db_network_->dbToSta(modnet_out2);
ASSERT_NE(sta_modnet_out2, nullptr);
// highestNetAbove on a mod net now returns the associated flat dbNet, not the
// highest mod net, so that parasitic externality checks work correctly.
Net* sta_highest_modnet_out
= db_network_->highestNetAbove(sta_modnet_mod_out);
ASSERT_EQ(sta_highest_modnet_out, sta_modnet_out2);
ASSERT_EQ(sta_highest_modnet_out, sta_dbnet_out2);

// Check get_ports -of_object Net*
NetTermIterator* term_iter = db_network_->termIterator(sta_dbnet_out2);
Expand Down
2 changes: 1 addition & 1 deletion src/sta
Open in desktop
Submodule sta updated 0 files
Loading