From fd61a6aa4a9a7ff6bb0ecf89b32091bf231c3de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Harboe?= Date: Wed, 7 Oct 2026 08:16:21 +0200 Subject: [PATCH] est: make OpenSTA's parasitic entries before making parasitics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Øyvind Harboe --- src/dbSta/test/cpp/TestLambertW.cc | 10 +++++++- src/est/BUILD | 1 + src/est/include/est/EstimateParasitics.h | 2 ++ src/est/src/EstimateParasitics.cpp | 27 ++++++++++++++++++-- src/est/src/MakeWireParasitics.cpp | 2 ++ src/est/src/StaParasiticsEntries.h | 32 ++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 src/est/src/StaParasiticsEntries.h diff --git a/src/dbSta/test/cpp/TestLambertW.cc b/src/dbSta/test/cpp/TestLambertW.cc index eb238d60c09..c04c32b7c9d 100644 --- a/src/dbSta/test/cpp/TestLambertW.cc +++ b/src/dbSta/test/cpp/TestLambertW.cc @@ -67,7 +67,15 @@ TEST_F(TestLambertW, ValidateRandomPImodels) std::uniform_real_distribution dis_c1(1e-15f, 50e-15f); std::uniform_real_distribution dis_slew(0.01e-9f, 0.5e-9f); - const Net* net = db_network_->net(drvr_pin); + // The net the parasitics store keys drvr_pin's network by. + const Net* net = db_network_->highestConnectedNet(db_network_->net(drvr_pin)); + // OpenSTA's lock-free parasitics store needs an entry for the driver + // before a network is made; older OpenSTA has no such call. + [&](P* p) { + if constexpr (requires { p->ensureParasitics(drvr_pin); }) { + p->ensureParasitics(drvr_pin); + } + }(parasitics); ASSERT_NE(net, nullptr); static constexpr int kNumSamples = 10000; diff --git a/src/est/BUILD b/src/est/BUILD index 8addc84cc5b..68603b1d339 100644 --- a/src/est/BUILD +++ b/src/est/BUILD @@ -35,6 +35,7 @@ cc_library( "src/AbstractSteinerRenderer.h", "src/MakeWireParasitics.h", "src/OdbCallBack.h", + "src/StaParasiticsEntries.h", ], copts = [ "-fopenmp", diff --git a/src/est/include/est/EstimateParasitics.h b/src/est/include/est/EstimateParasitics.h index 8ee29a9895e..eb951f382b7 100644 --- a/src/est/include/est/EstimateParasitics.h +++ b/src/est/include/est/EstimateParasitics.h @@ -244,6 +244,8 @@ class EstimateParasitics : public sta::dbStaState, public ParasiticsService template const std::vector& resolveWireRC(std::vector WireRC::*category) const; void ensureParasitics(); + // Give each driver of net an entry in every scene's parasitics. + void makeParasiticsEntries(const sta::Net* net); bool isSkipPin(const sta::Pin* pin) const; bool hasIdealClocks(const sta::Mode* mode) const; bool hasIdealClocks() const; diff --git a/src/est/src/EstimateParasitics.cpp b/src/est/src/EstimateParasitics.cpp index bb54db1ef8f..1e0a42222c2 100644 --- a/src/est/src/EstimateParasitics.cpp +++ b/src/est/src/EstimateParasitics.cpp @@ -21,6 +21,7 @@ #include "AbstractSteinerRenderer.h" #include "MakeWireParasitics.h" #include "OdbCallBack.h" +#include "StaParasiticsEntries.h" #include "db_sta/SpefWriter.hh" #include "db_sta/dbNetwork.hh" #include "db_sta/dbSta.hh" @@ -160,6 +161,7 @@ void EstimateParasitics::updateGlobalRouteParasitics(odb::dbNet* db_net, if (db_net == nullptr || route.empty()) { return; } + makeParasiticsEntries(db_network_->dbToSta(db_net)); estimateGlobalRouteParasitics(db_net, route); // Annotating parasitics alone leaves stale cached delays in the timer. sta_->delaysInvalidFromFanin(db_network_->dbToSta(db_net)); @@ -653,6 +655,7 @@ void EstimateParasitics::ensureWireParasitic(const sta::Pin* drvr_pin) void EstimateParasitics::ensureWireParasitic(const sta::Pin* drvr_pin, const sta::Net* net) { + makeParasiticsEntries(net); // Sufficient to check for parasitic for one corner because // they are all made at the same time. const Scene* corner = sta_->scenes().front(); @@ -686,6 +689,7 @@ void EstimateParasitics::estimateGlobalRouteRC(sta::SpefWriter* spef_writer) for (Scene* scene : scenes_) { Parasitics* parasitics = sta_->makeConcreteParasitics(scene->name(), ""); scene->setParasitics(parasitics, sta::MinMaxAll::minMax()); + est::makeParasiticsEntries(parasitics); } auto& routes = global_router_->getRoutes(); @@ -715,6 +719,7 @@ void EstimateParasitics::estimateGlobalRouteRC(sta::SpefWriter* spef_writer) void EstimateParasitics::estimateGlobalRouteRC(odb::dbNet* db_net) { + makeParasiticsEntries(db_network_->dbToSta(db_net)); MakeWireParasitics builder( logger_, this, sta_, block_->getTech(), block_, global_router_); auto& routes = global_router_->getRoutes(); @@ -735,6 +740,7 @@ void EstimateParasitics::estimateGlobalRouteParasitics(odb::dbNet* net, return; } initBlock(); + makeParasiticsEntries(db_network_->dbToSta(net)); MakeWireParasitics builder( logger_, this, sta_, block_->getTech(), block_, global_router_); @@ -768,6 +774,7 @@ void EstimateParasitics::estimateWireParasitics(sta::SpefWriter* spef_writer) for (Scene* scene : scenes_) { Parasitics* parasitics = sta_->makeConcreteParasitics(scene->name(), ""); scene->setParasitics(parasitics, sta::MinMaxAll::minMax()); + est::makeParasiticsEntries(parasitics); } sta::LibertyLibrary* default_lib = network_->defaultLibertyLibrary(); @@ -829,6 +836,7 @@ void EstimateParasitics::estimateWireParasitic(const sta::Net* net, { PinSet* drivers = network_->drivers(net); if (drivers && !drivers->empty()) { + makeParasiticsEntries(net); const Pin* drvr_pin = *drivers->begin(); estimateWireParasitic(drvr_pin, net, spef_writer); } @@ -863,6 +871,7 @@ void EstimateParasitics::makeWireParasitic(sta::Net* net, const Scene* corner) { sta::Parasitics* parasitics = corner->parasitics(max_); + est::makeParasiticsEntry(parasitics, drvr_pin); sta::Parasitic* parasitic = parasitics->makeParasiticNetwork(net, false); sta::ParasiticNode* n1 = parasitics->ensureParasiticNode(parasitic, drvr_pin, network_); @@ -1354,6 +1363,19 @@ bool EstimateParasitics::isPad(const sta::Instance* inst) const return false; } +void EstimateParasitics::makeParasiticsEntries(const sta::Net* net) +{ + PinSet* drivers = network_->drivers(net); + if (drivers == nullptr) { + return; + } + for (const sta::Scene* scene : sta_->scenes()) { + for (const sta::Pin* drvr_pin : *drivers) { + est::makeParasiticsEntry(scene->parasitics(max_), drvr_pin); + } + } +} + bool EstimateParasitics::isSkipPin(const sta::Pin* pin) const { // A pin can be skipped when its parasitics never affect timing in any mode. @@ -1485,11 +1507,12 @@ void EstimateParasitics::eraseParasitics(const sta::Net* net) // // deleteParasiticNetwork() and not deleteParasitics(): the latter resolves // drivers(net), which by now has no driver and would cache an empty entry - // keyed on a dbNet about to be freed. + // keyed on a dbNet about to be freed. A net with no entry has nothing to + // delete, and OpenSTA's deleteParasiticNetwork needs the entry to exist. for (Scene* scene : scenes_) { for (const sta::MinMax* mm : sta::MinMax::range()) { Parasitics* parasitics = scene->parasitics(mm); - if (parasitics) { + if (parasitics && parasitics->findParasiticNetwork(net)) { parasitics->deleteParasiticNetwork(net); } } diff --git a/src/est/src/MakeWireParasitics.cpp b/src/est/src/MakeWireParasitics.cpp index 27724c1cd5f..badfac228c8 100644 --- a/src/est/src/MakeWireParasitics.cpp +++ b/src/est/src/MakeWireParasitics.cpp @@ -9,6 +9,7 @@ #include #include +#include "StaParasiticsEntries.h" #include "db_sta/SpefWriter.hh" #include "db_sta/dbNetwork.hh" #include "db_sta/dbSta.hh" @@ -153,6 +154,7 @@ void MakeWireParasitics::clearParasitics() for (Scene* scene : sta_->scenes()) { Parasitics* parasitics = sta_->makeConcreteParasitics(scene->name(), ""); scene->setParasitics(parasitics, sta::MinMaxAll::minMax()); + makeParasiticsEntries(parasitics); } } diff --git a/src/est/src/StaParasiticsEntries.h b/src/est/src/StaParasiticsEntries.h new file mode 100644 index 00000000000..904e4190f09 --- /dev/null +++ b/src/est/src/StaParasiticsEntries.h @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2026, The OpenROAD Authors + +#pragma once + +#include "sta/NetworkClass.hh" +#include "sta/Parasitics.hh" + +namespace est { + +// OpenSTA's parasitics store finds and makes parasitics without a lock +// once every driver and its net have an entry, and makeParasiticNetwork +// no longer adds the entry itself. OpenSTA before that change has +// neither call and adds the entry under its own lock, so there is +// nothing to do. Call these only where no other thread uses the store. +template +void makeParasiticsEntries(Parasitics* parasitics) +{ + if constexpr (requires { parasitics->ensureParasitics(); }) { + parasitics->ensureParasitics(); + } +} + +template +void makeParasiticsEntry(Parasitics* parasitics, const sta::Pin* drvr_pin) +{ + if constexpr (requires { parasitics->ensureParasitics(drvr_pin); }) { + parasitics->ensureParasitics(drvr_pin); + } +} + +} // namespace est