Skip to content

Commit 16febd5

Browse files
jhkim-piijhkim-pii
authored
Merge pull request #12030 from The-OpenROAD-Project-staging/rsz-compute-skew-restore-fix
rsz: preserve asymmetric pin clock latency in computeSkewBudget
2 parents 1740e32 + b3b996e commit 16febd5

3 files changed

Lines changed: 194 additions & 22 deletions

File tree

‎src/rsz/src/Resizer.cc‎

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5905,7 +5905,14 @@ void Resizer::computeSkewBudget(const SkewBudgetOptions& opts)
59055905
sta_->findRequireds();
59065906
}
59075907
std::vector<float> initial_latency(ck_pins.size(), 0.0f);
5908-
std::vector<bool> initial_pin_latency_set(ck_pins.size(), false);
5908+
// Full original pin-level latency per (rise/fall, min/max) so an asymmetric
5909+
// annotation is restored exactly instead of being collapsed to one value.
5910+
// initial_latency[] keeps only the rise/max baseline the LP shifts.
5911+
using PinLatency = std::array<std::array<float, 2>, 2>;
5912+
using PinLatencySet = std::array<std::array<bool, 2>, 2>;
5913+
std::vector<PinLatency> orig_pin_latency(ck_pins.size(), PinLatency{});
5914+
std::vector<PinLatencySet> orig_pin_latency_set(ck_pins.size(),
5915+
PinLatencySet{});
59095916
for (int i = 0; i < static_cast<int>(ck_pins.size()); ++i) {
59105917
if (ck_propagated[i]) {
59115918
// Max rise *clock-path* arrival at the CK pin. Sta::arrival()
@@ -5938,11 +5945,19 @@ void Resizer::computeSkewBudget(const SkewBudgetOptions& opts)
59385945
float val = 0.0f;
59395946
bool exists = false;
59405947
sdc->clockLatency(ck_clocks[i], ck_pins[i], rf_rise, mm_max, val, exists);
5941-
initial_pin_latency_set[i] = exists;
59425948
if (!exists) {
59435949
sdc->clockLatency(ck_clocks[i], rf_rise, mm_max, val, exists);
59445950
}
59455951
initial_latency[i] = val;
5952+
for (const sta::RiseFall* rf : sta::RiseFall::range()) {
5953+
for (const sta::MinMax* mm : sta::MinMax::range()) {
5954+
float lat = 0.0f;
5955+
bool lat_exists = false;
5956+
sdc->clockLatency(ck_clocks[i], ck_pins[i], rf, mm, lat, lat_exists);
5957+
orig_pin_latency_set[i][rf->index()][mm->index()] = lat_exists;
5958+
orig_pin_latency[i][rf->index()][mm->index()] = lat;
5959+
}
5960+
}
59465961
}
59475962

59485963
// 4. Cutting-plane iteration.
@@ -6358,18 +6373,37 @@ void Resizer::computeSkewBudget(const SkewBudgetOptions& opts)
63586373
// budget never leaves the cutting-plane's tentative set_clock_latency
63596374
// annotations in the SDC. Without this, an abort would corrupt downstream
63606375
// timing with skew that was computed but never realized.
6376+
// Put one CK pin back to its exact pre-LP SDC state: drop the clock/pin
6377+
// latency the LP wrote, then re-apply each original per-(rise/fall, min/max)
6378+
// value only if removing the LP's annotation also dropped it. A
6379+
// clock-agnostic pin latency survives removeClockLatency, so re-applying it
6380+
// would duplicate the original as a clock-specific annotation.
6381+
auto restore_pin_latency = [&](int i) {
6382+
sta_->removeClockLatency(ck_clocks[i], ck_pins[i], sdc);
6383+
for (const sta::RiseFall* rf : sta::RiseFall::range()) {
6384+
for (const sta::MinMax* mm : sta::MinMax::range()) {
6385+
const size_t ri = rf->index();
6386+
const size_t mi = mm->index();
6387+
if (!orig_pin_latency_set[i][ri][mi]) {
6388+
continue;
6389+
}
6390+
float cur = 0.0f;
6391+
bool cur_exists = false;
6392+
sdc->clockLatency(ck_clocks[i], ck_pins[i], rf, mm, cur, cur_exists);
6393+
if (!cur_exists || cur != orig_pin_latency[i][ri][mi]) {
6394+
sta_->setClockLatency(ck_clocks[i],
6395+
const_cast<sta::Pin*>(ck_pins[i]),
6396+
rf->asRiseFallBoth(),
6397+
mm->asMinMaxAll(),
6398+
orig_pin_latency[i][ri][mi],
6399+
sdc);
6400+
}
6401+
}
6402+
}
6403+
};
63616404
auto restore_initial_latencies = [&]() {
63626405
for (int i = 0; i < static_cast<int>(ck_pins.size()); ++i) {
6363-
if (initial_pin_latency_set[i]) {
6364-
sta_->setClockLatency(ck_clocks[i],
6365-
const_cast<sta::Pin*>(ck_pins[i]),
6366-
sta::RiseFallBoth::riseFall(),
6367-
sta::MinMaxAll::all(),
6368-
initial_latency[i],
6369-
sdc);
6370-
} else {
6371-
sta_->removeClockLatency(ck_clocks[i], ck_pins[i], sdc);
6372-
}
6406+
restore_pin_latency(i);
63736407
}
63746408
sta_->findRequireds();
63756409
};
@@ -6555,16 +6589,7 @@ void Resizer::computeSkewBudget(const SkewBudgetOptions& opts)
65556589
// and double-count once CTS inserts the delay buffers). The schedule
65566590
// survives only as the useful_skew_target property — a timing-inert
65576591
// signal CTS reads to decide whether to realize the skew.
6558-
if (initial_pin_latency_set[i]) {
6559-
sta_->setClockLatency(ck_clocks[i],
6560-
const_cast<sta::Pin*>(ck_pins[i]),
6561-
sta::RiseFallBoth::riseFall(),
6562-
sta::MinMaxAll::all(),
6563-
initial_latency[i],
6564-
sdc);
6565-
} else {
6566-
sta_->removeClockLatency(ck_clocks[i], ck_pins[i], sdc);
6567-
}
6592+
restore_pin_latency(i);
65686593

65696594
if (std::fabs(ti) >= opts.emit_threshold) {
65706595
odb::dbITerm* ck_iterm = nullptr;

‎src/rsz/test/BUILD‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,30 @@ cc_test(
747747
],
748748
)
749749

750+
cc_test(
751+
name = "TestSkewBudgetRestore",
752+
size = "small",
753+
srcs = ["cpp/TestSkewBudgetRestore.cc"],
754+
copts = ["-Isrc/rsz/src"],
755+
data = [
756+
"Nangate45/Nangate45.lef",
757+
"Nangate45/Nangate45_typ.lib",
758+
"repair_hold1.def",
759+
],
760+
deps = [
761+
"//src/dbSta",
762+
"//src/dbSta:dbNetwork",
763+
"//src/est",
764+
"//src/odb/src/db",
765+
"//src/odb/src/defin",
766+
"//src/rsz",
767+
"//src/sta:opensta_lib",
768+
"//src/tst:integrated_fixture",
769+
"@googletest//:gtest",
770+
"@googletest//:gtest_main",
771+
],
772+
)
773+
750774
py_library(
751775
name = "helpers",
752776
srcs = ["helpers.py"],

‎src/rsz/test/cpp/TestSkewBudgetRestore.cc‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
// Copyright (c) 2026-2026, The OpenROAD Authors
3+
4+
// compute_skew_budget shifts register clock latencies during its LP and then
5+
// restores the original annotations. A pin whose set_clock_latency is
6+
// asymmetric (min != max) must come back unchanged; a regression once
7+
// collapsed it to a single value, corrupting the hold (min) latency.
8+
9+
#include <vector>
10+
11+
#include "db_sta/dbNetwork.hh"
12+
#include "db_sta/dbSta.hh"
13+
#include "est/EstimateParasitics.h"
14+
#include "gtest/gtest.h"
15+
#include "odb/db.h"
16+
#include "odb/defin.h"
17+
#include "rsz/Resizer.hh"
18+
#include "sta/Clock.hh"
19+
#include "sta/MinMax.hh"
20+
#include "sta/Mode.hh"
21+
#include "sta/NetworkClass.hh"
22+
#include "sta/Sdc.hh"
23+
#include "sta/Transition.hh"
24+
#include "sta/Units.hh"
25+
#include "tst/IntegratedFixture.h"
26+
27+
namespace rsz {
28+
29+
class SkewBudgetRestoreTest : public tst::IntegratedFixture
30+
{
31+
protected:
32+
SkewBudgetRestoreTest()
33+
: tst::IntegratedFixture(tst::IntegratedFixture::Technology::kNangate45,
34+
"_main/src/rsz/test/")
35+
{
36+
}
37+
38+
void SetUp() override
39+
{
40+
odb::dbChip* chip = odb::dbChip::create(db_.get(), db_->getTech());
41+
odb::defin def_reader(db_.get(), &logger_, odb::defin::DEFAULT);
42+
std::vector<odb::dbLib*> search_libs;
43+
for (odb::dbLib* lib : db_->getLibs()) {
44+
search_libs.push_back(lib);
45+
}
46+
def_reader.readChip(
47+
search_libs,
48+
getFilePath("_main/src/rsz/test/repair_hold1.def").c_str(),
49+
chip);
50+
block_ = db_->getChip()->getBlock();
51+
sta_->postReadDef(block_);
52+
53+
// create_clock -period 2 clk
54+
sta::PinSet clk_pins(db_network_);
55+
clk_pins.insert(db_network_->findPin("clk"));
56+
const float period = ns(2.0);
57+
sta_->makeClock("clk",
58+
clk_pins,
59+
/*add_to_pins=*/false,
60+
period,
61+
{0, period / 2},
62+
/*comment=*/"",
63+
sta_->cmdMode());
64+
}
65+
66+
float ns(const double value) const
67+
{
68+
return sta_->units()->timeUnit()->userToSta(value);
69+
}
70+
71+
float pinLatency(sta::Clock* clk, sta::Pin* pin, const sta::MinMax* min_max)
72+
{
73+
float latency = 0.0f;
74+
bool exists = false;
75+
sta_->cmdMode()->sdc()->clockLatency(
76+
clk, pin, sta::RiseFall::rise(), min_max, latency, exists);
77+
EXPECT_TRUE(exists);
78+
return latency;
79+
}
80+
};
81+
82+
TEST_F(SkewBudgetRestoreTest, AsymmetricPinLatencyPreserved)
83+
{
84+
sta::Sdc* sdc = sta_->cmdMode()->sdc();
85+
sta::Pin* ck = db_network_->findPin("r3/CK");
86+
ASSERT_NE(ck, nullptr);
87+
sta::Clock* clk = sdc->findClock("clk");
88+
ASSERT_NE(clk, nullptr);
89+
90+
// set_clock_latency -rise -min 0.02 / -max 0.10 on r3/CK
91+
const float min_latency = ns(0.02);
92+
const float max_latency = ns(0.10);
93+
sta_->setClockLatency(clk,
94+
ck,
95+
sta::RiseFallBoth::rise(),
96+
sta::MinMaxAll::min(),
97+
min_latency,
98+
sdc);
99+
sta_->setClockLatency(clk,
100+
ck,
101+
sta::RiseFallBoth::rise(),
102+
sta::MinMaxAll::max(),
103+
max_latency,
104+
sdc);
105+
106+
const float min_before = pinLatency(clk, ck, sta::MinMax::min());
107+
const float max_before = pinLatency(clk, ck, sta::MinMax::max());
108+
109+
sta_->ensureGraph();
110+
sta_->ensureLevelized();
111+
resizer_.initBlock();
112+
ep_.estimateWireParasitics();
113+
114+
Resizer::SkewBudgetOptions opts;
115+
opts.max_iterations = 1;
116+
resizer_.computeSkewBudget(opts);
117+
118+
// The asymmetric annotation must survive unchanged.
119+
EXPECT_FLOAT_EQ(pinLatency(clk, ck, sta::MinMax::min()), min_before);
120+
EXPECT_FLOAT_EQ(pinLatency(clk, ck, sta::MinMax::max()), max_before);
121+
}
122+
123+
} // namespace rsz

0 commit comments

Comments
 (0)