Repository navigation
Conversation
OpenSTA's parasitics store now finds and makes parasitics without a lock (parallaxsw/OpenSTA a54fb953..b7d866ff). For that, every driver and its net need an entry, made by Parasitics::ensureParasitics before any thread uses the store, and makeParasiticNetwork no longer adds a missing one. est makes its own stores and never calls it, so the first estimate_parasitics after the next OpenSTA bump segfaults in makeParasiticNetwork, even at one thread. est now makes the entries for the whole design after each new store, before the parallel loops, and for a net's drivers on the serial single-net paths that may see a net made after the store. A net with no entry is not deleted, since deleteParasiticNetwork needs the entry. TestLambertW makes its driver's entry and builds on the net the store keys it by. The calls go through a requires-expression, so with the OpenSTA OpenROAD pins today they compile to nothing and nothing changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates with OpenSTA's lock-free parasitics store by ensuring parasitics entries are created for net drivers before networks are built. It introduces a helper header StaParasiticsEntries.h and updates EstimateParasitics and MakeWireParasitics to invoke these helpers during parasitic estimation and routing updates. The review feedback focuses on improving robustness by adding nullptr checks for parasitics, drvr_pin, and net across several of the newly introduced or modified functions to prevent potential segmentation faults.
|
@jhkim-pii @maliberty @eder-matheus without changes, OpenROAD will crash on next opensta upgrade, this is a future proofing PR. Anything more needed from me? |
|
The pr-merge CI failure is unrelated and should fix itself when it runs next. |
|
Some Claude review:
|
|
@eder-matheus Can you take it from here? |
|
@maliberty OpenROAD will crash on next opensta bump, this PR/patch works for me meanwhile. I think it is more expedient if you handle the opensta bump. |
Yeah, I can apply the suggested changes from here once we bump opensta version. |
|
@eder-matheus Thank you! I'm sure there will be a couple of things to iron out! 😅 |
|
Already in the process of being merged STA PR The-OpenROAD-Project/OpenSTA#422 |
OpenSTA's parasitics store now finds and makes parasitics without a lock (parallaxsw/OpenSTA a54fb953..b7d866ff). It needs an entry for every driver and its net made up front, by
Parasitics::ensureParasitics, andmakeParasiticNetworkno longer adds a missing one. est makes its own stores and never makes the entries, so after the next OpenSTA bump the firstestimate_parasiticssegfaults inmakeParasiticNetwork, even at one thread.This PR makes est create the entries, so the bump can go in without a crash. With the OpenSTA OpenROAD pins today, it changes nothing.
What
estimateWireParasitics,estimateGlobalRouteRC,MakeWireParasitics::clearParasitics): make the entries for the whole design, before the parallel loops.ensureWireParasitic,estimateWireParasitic(net),estimateGlobalRouteRC(net),estimateGlobalRouteParasitics,updateGlobalRouteParasitics,makeWireParasitic): make the entries for the net's drivers, since the net may have been made after the store (repair_design's buffers).eraseParasitics: skip a net with no entry, sincedeleteParasiticNetworknow needs one.TestLambertW: make the driver's entry, and build on the net the store keys it by (highestConnectedNet).The calls go through a
requiresexpression insrc/est/src/StaParasiticsEntries.h. Against today's OpenSTA they compile to nothing; once the bump is in, they can become plain calls.Hierarchical designs need an OpenSTA fix too
In
-hiermode, two places in OpenSTA make an entry under a different net than est uses: the top-level-port branch offindParasiticNet, andread_spef -path. That crashespinswap_hier,repair_fanout1_hierandread_vcd, and est cannot fix it. It is filed as parallaxsw/OpenSTA#524.Tests
The
est,rsz,grt,ctsanddbStatests (3076), plus a stress script (aes: 20×estimate_parasitics -placementat 16 threads compared pin by pin with 1 thread, thenrepair_design,repair_timing):e983e15b)est_placement_threadsand the stress scriptreport_checks -group_path_count 200 -format end -digits 6after the stress script is byte-identical to master in every passing row. No new test: with today's OpenSTA there is nothing to observe, and after the bump the tests above cover it.The lock removal in OpenSTA is the heavy lifting; this only keeps est in step with it. Claude made the changes and stared at paint drying, which is how beta-testing works these days.
🤖 Generated with Claude Code