est: delete the parasitic network when its dbNet is destroyed - #11227
Conversation
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
There was a problem hiding this comment.
Code Review
This pull request ensures that destroying a dbNet properly deletes its associated parasitic network annotation. Previously, dbNet::destroy() only disconnected pin nodes, leaving a driver-less network on the dead net, which could then be inherited by a new net when the dbNet slot was recycled. The fix explicitly deletes the parasitic network in EstimateParasitics::eraseParasitics. A new regression test prima_net_recycle has been added to verify this behavior, along with its corresponding test files and build targets. There are no review comments, and the implementation is clean and well-tested.
eder-matheus
left a comment
There was a problem hiding this comment.
@dsengupta0628 Does it need a secure-ci run? You mentioned wrong values being calculated on the NLDM path. Perhaps this could affect some designs.
Hi Eder. Yes I plan to run the full ORFS. I didn’t do it yet as it was very late last night by the time I filed this PR :) |
The-OpenROAD-Project/OpenROAD-flow-scripts#4472 shows all clean |
Summary
Fixes #11192. Fixes the crashes reported in #11178.
Problem
sta::Net*is thedbNet*(dbNetwork::dbToStais areinterpret_cast), andConcreteParasiticskeysparasitic_network_map_by it. Destroying adbNetonly disconnected the parasitic network's pin nodes — the network itself stayed annotated on the dead net, becauseEstimateParasitics::eraseParasitics()only erased the net fromparasitics_invalid_.odbrecyclesdbNetslots, so the next net created lands on the samesta::Net*and inherits a network that has wire nodes but no node for its driver pin.flowchart TD A["repair_timing edits netlist<br/>dbNet::destroy()"] --> B["inDbNetDestroy() fires"] B --> C["eraseParasitics() clears only<br/>the parasitics_invalid_ marker"] C --> D["Network stays in parasitic_network_map_,<br/>pin nodes already disconnected"] D --> E["odb recycles the dbNet slot"] E --> F["New net = same sta::Net*<br/>inherits a driver-less network"] F --> G{"Delay calculator"} G -->|table| H["Reduces it to pi-Elmore.<br/>Silently wrong, no crash."] G -->|prima| I["No driver node<br/>node_count_ == 0"] I --> J["Zero-node circuit:<br/>empty threshold_times_,<br/>singular matrix → SIGSEGV"]Only CCS/prima runs crash. The table calculator reduces the stale network and never inspects individual nodes, so it just uses wrong numbers.
Both stacks in #11178 reach this through the same middle section — the incremental router re-entering the timer — differing only in the resizer entry point:
Why this fix is in OpenROAD, not OpenSTA
sta::Net*-is-dbNet*is a dbSta invention and slot recycling isodb's. Upstream OpenSTA has no notion of one net pointer later meaning a different net; teaching it that would export an OpenROAD-specific representation upstream.estowns these annotations. They are produced byestimate_parasitics, not read from SPEF.estalready had a net-destroy hook,eraseParasitics(), meant for exactly this cleanup — it just did too little.sta::Parasiticsis a passive store.Fix
eraseParasitics()now deletes the annotation across all scenes, min and max.It calls
deleteParasiticNetwork()and notdeleteParasitics(): the latter resolvesdrivers(net), anddbNet::destroy()has already disconnected every iterm by the timeinDbNetDestroy()fires, so that lookup finds no driver and re-populatesdbNetwork's driver cache with an empty entry keyed on adbNetthat is about to be freed.Test
src/est/test/prima_net_recycle- 6 instances, one net destroyed and rebuilt with an identical pin set. Crashes before the fix inPrimaDelayCalc::measureThresholds-->simulate1; passes after this fix.Two conditions needed to be met, and hence commented in the test:
primamust be selected beforeestimate_parasitics, or the default calculator reduces each network to pi-Elmore and deletes it, leaving nothing to inherit.total_output_net_capacitance), orPrimaDelayCalc::checkArgs()falls back to the table calculator and the test is vacuous. Hence the extra inverter loads.Adds
test/asap7/asap7_small_ccs.lib.gzwith public filegroups//test:asap7_ccs_dataand//test:asap7_data. OpenROAD had no asap7 CCS liberty outside thesrc/stasubmodule, and the shared asap7 libs are NLDM only, soprimacould not be exercised from a module test before this.Type of Change
Impact
[How does this change the tool's behavior?]
Verification
./etc/Build.sh).Related Issues
#11178
#11192