Skip to content

Commit 19294a8

Browse files
malibertymaliberty
authored
Merge pull request #8901 from The-OpenROAD-Project-staging/secure-change-undriven-buffer-handling
Changed RSZ-168 from ERROR to WARNING in `UnbufferMove::removeBuffer()`
2 parents f46913d + 4e2532c commit 19294a8

File tree

7 files changed

+264
-17
lines changed

7 files changed

+264
-17
lines changed

‎src/rsz/src/UnbufferMove.cc‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ bool UnbufferMove::doMove(const Path* drvr_path,
195195
1,
196196
"ACCEPT unbuffer {}",
197197
network_->pathName(drvr));
198-
removeBuffer(drvr);
199-
return true;
198+
return removeBuffer(drvr);
200199
}
201200
debugPrint(logger_,
202201
RSZ,
@@ -213,8 +212,7 @@ bool UnbufferMove::removeBufferIfPossible(Instance* buffer,
213212
bool honorDontTouchFixed)
214213
{
215214
if (canRemoveBuffer(buffer, honorDontTouchFixed)) {
216-
removeBuffer(buffer);
217-
return true;
215+
return removeBuffer(buffer);
218216
}
219217
return false;
220218
}
@@ -327,7 +325,7 @@ bool UnbufferMove::canRemoveBuffer(Instance* buffer, bool honorDontTouchFixed)
327325
return false;
328326
}
329327

330-
void UnbufferMove::removeBuffer(Instance* buffer)
328+
bool UnbufferMove::removeBuffer(Instance* buffer)
331329
{
332330
LibertyCell* lib_cell = network_->libertyCell(buffer);
333331
debugPrint(logger_,
@@ -338,34 +336,32 @@ void UnbufferMove::removeBuffer(Instance* buffer)
338336
network_->pathName(buffer),
339337
lib_cell->name());
340338

341-
addMove(buffer);
342-
343339
LibertyPort *in_port, *out_port;
344340
lib_cell->bufferPorts(in_port, out_port);
345341

346342
Pin* in_pin = db_network_->findPin(buffer, in_port);
347343
Pin* out_pin = db_network_->findPin(buffer, out_port);
348344

349-
// Hierarchical net handling
350-
odb::dbModNet* op_modnet = db_network_->hierNet(out_pin);
351-
odb::dbModNet* ip_modnet = db_network_->hierNet(in_pin);
352-
353345
odb::dbNet* in_db_net = db_network_->flatNet(in_pin);
354346
odb::dbNet* out_db_net = db_network_->flatNet(out_pin);
355347

356348
// Handle undriven buffer
357349
if (in_db_net == nullptr) {
358-
logger_->error(RSZ,
359-
168,
360-
"Cannot remove undriven buffer {}.",
361-
network_->pathName(buffer));
350+
logger_->warn(
351+
RSZ,
352+
168,
353+
"The input pin of buffer '{}' is undriven. Do not remove the buffer.",
354+
network_->pathName(buffer));
355+
return false;
362356
}
363357

358+
addMove(buffer);
359+
364360
// Remove the unused buffer
365361
if (out_db_net == nullptr) {
366362
dbInst* dbinst_buffer = db_network_->staToDb(buffer);
367363
dbInst::destroy(dbinst_buffer);
368-
return;
364+
return true;
369365
}
370366

371367
// in_net and out_net are flat nets.
@@ -375,6 +371,8 @@ void UnbufferMove::removeBuffer(Instance* buffer)
375371
// Decide survivor net when two nets are merged
376372
Net* survivor = in_net; // buffer input net is the default survivor
377373
Net* removed = out_net;
374+
odb::dbModNet* op_modnet = db_network_->hierNet(out_pin);
375+
odb::dbModNet* ip_modnet = db_network_->hierNet(in_pin);
378376
odb::dbModNet* survivor_modnet = ip_modnet;
379377
odb::dbModNet* removed_modnet = op_modnet;
380378
bool in_net_has_port = db_network_->hasPort(in_net);
@@ -453,6 +451,8 @@ void UnbufferMove::removeBuffer(Instance* buffer)
453451
if (survivor_modnet != nullptr && new_modnet_name) {
454452
survivor_modnet->rename(new_modnet_name->c_str());
455453
}
454+
455+
return true;
456456
}
457457

458458
} // namespace rsz

‎src/rsz/src/UnbufferMove.hh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class UnbufferMove : public BaseMove
3030

3131
bool removeBufferIfPossible(Instance* buffer, bool honorDontTouchFixed);
3232
bool canRemoveBuffer(Instance* buffer, bool honorDontTouchFixed);
33-
void removeBuffer(Instance* buffer);
33+
bool removeBuffer(Instance* buffer);
3434

3535
private:
3636
bool bufferBetweenPorts(Instance* buffer);

‎src/rsz/test/BUILD‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,9 @@ cc_test(
322322
"cpp/TestBufferRemoval3_4.v",
323323
"cpp/TestBufferRemoval3_5.v",
324324
"cpp/TestBufferRemoval3_6.v",
325+
"cpp/TestBufferRemoval3_7.v",
326+
"cpp/TestBufferRemoval3_8.v",
327+
"cpp/TestBufferRemoval3_9.v",
325328
],
326329
deps = [
327330
"//src/dbSta",

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

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
#include <memory>
88
#include <string>
99

10+
#include "db_sta/dbSta.hh"
1011
#include "gtest/gtest.h"
1112
#include "odb/db.h"
13+
#include "sta/NetworkClass.hh"
1214
#include "sta/VerilogWriter.hh"
1315
#include "tst/IntegratedFixture.h"
1416
#include "utl/Logger.h"
@@ -30,6 +32,212 @@ class BufRemTest3 : public tst::IntegratedFixture
3032
bool debug_ = false; // Set to true to generate debug output
3133
};
3234

35+
TEST_F(BufRemTest3, RemoveBufferCase9)
36+
{
37+
std::string test_name = "TestBufferRemoval3_9";
38+
readVerilogAndSetup(test_name + ".v");
39+
40+
// Netlist before buffer removal:
41+
// (undriven input) -> buf1 -> out
42+
43+
// Dump pre ECO state
44+
if (debug_) {
45+
dumpVerilogAndOdb(test_name + "_pre_eco");
46+
}
47+
48+
odb::dbDatabase::beginEco(block_);
49+
50+
// Pre sanity check
51+
sta_->updateTiming(true);
52+
// Do not call checkAxioms() because there is an undriven buffer.
53+
54+
//----------------------------------------------------
55+
// Remove buffer
56+
//----------------------------------------------------
57+
auto insts = std::make_unique<sta::InstanceSeq>();
58+
odb::dbInst* buf_inst = block_->findInst("buf1");
59+
ASSERT_NE(buf_inst, nullptr);
60+
sta::Instance* sta_buf = db_network_->dbToSta(buf_inst);
61+
insts->emplace_back(sta_buf);
62+
resizer_.removeBuffers(*insts);
63+
64+
// Post sanity check
65+
sta_->updateTiming(true);
66+
// Do not call checkAxioms() because there is an undriven buffer.
67+
68+
// Write verilog and check the content after buffer removal
69+
const std::string after_vlog_path = test_name + "_after.v";
70+
sta::writeVerilog(after_vlog_path.c_str(), true, false, {}, sta_->network());
71+
72+
std::ifstream file_after(after_vlog_path);
73+
std::string content_after((std::istreambuf_iterator<char>(file_after)),
74+
std::istreambuf_iterator<char>());
75+
76+
// Netlist after buffer removal:
77+
// in -> mod_inst/mod_in -> assign -> mod_inst/mod_out -> out
78+
const std::string expected_after_vlog = R"(module top (clk,
79+
in,
80+
out);
81+
input clk;
82+
input in;
83+
output out;
84+
85+
86+
endmodule
87+
)";
88+
89+
EXPECT_EQ(content_after, expected_after_vlog);
90+
91+
odb::dbDatabase::undoEco(block_);
92+
93+
// Dump undo ECO state
94+
if (debug_) {
95+
dumpVerilogAndOdb(test_name + "_undo_eco");
96+
}
97+
98+
// Clean up
99+
removeFile(after_vlog_path);
100+
}
101+
102+
TEST_F(BufRemTest3, RemoveBufferCase8)
103+
{
104+
std::string test_name = "TestBufferRemoval3_8";
105+
readVerilogAndSetup(test_name + ".v");
106+
107+
// Netlist before buffer removal:
108+
// (undriven input) -> buf1 -> out
109+
110+
// Dump pre ECO state
111+
if (debug_) {
112+
dumpVerilogAndOdb(test_name + "_pre_eco");
113+
}
114+
115+
odb::dbDatabase::beginEco(block_);
116+
117+
// Pre sanity check
118+
sta_->updateTiming(true);
119+
// Do not call checkAxioms() because there is an undriven buffer.
120+
121+
//----------------------------------------------------
122+
// Remove buffer
123+
//----------------------------------------------------
124+
auto insts = std::make_unique<sta::InstanceSeq>();
125+
odb::dbInst* buf_inst = block_->findInst("buf1");
126+
ASSERT_NE(buf_inst, nullptr);
127+
sta::Instance* sta_buf = db_network_->dbToSta(buf_inst);
128+
insts->emplace_back(sta_buf);
129+
resizer_.removeBuffers(*insts);
130+
131+
// Post sanity check
132+
sta_->updateTiming(true);
133+
// Do not call checkAxioms() because there is an undriven buffer.
134+
135+
// Write verilog and check the content after buffer removal
136+
const std::string after_vlog_path = test_name + "_after.v";
137+
sta::writeVerilog(after_vlog_path.c_str(), true, false, {}, sta_->network());
138+
139+
std::ifstream file_after(after_vlog_path);
140+
std::string content_after((std::istreambuf_iterator<char>(file_after)),
141+
std::istreambuf_iterator<char>());
142+
143+
// Netlist after buffer removal:
144+
// in -> mod_inst/mod_in -> assign -> mod_inst/mod_out -> out
145+
const std::string expected_after_vlog = R"(module top (clk,
146+
in,
147+
out);
148+
input clk;
149+
input in;
150+
output out;
151+
152+
153+
BUF_X1 buf1 (.Z(out));
154+
endmodule
155+
)";
156+
157+
EXPECT_EQ(content_after, expected_after_vlog);
158+
159+
odb::dbDatabase::undoEco(block_);
160+
161+
// Dump undo ECO state
162+
if (debug_) {
163+
dumpVerilogAndOdb(test_name + "_undo_eco");
164+
}
165+
166+
// Clean up
167+
removeFile(after_vlog_path);
168+
}
169+
170+
TEST_F(BufRemTest3, RemoveBufferCase7)
171+
{
172+
std::string test_name = "TestBufferRemoval3_7";
173+
readVerilogAndSetup(test_name + ".v");
174+
175+
// Netlist before buffer removal:
176+
// (undriven input) -> buf1 -> buf2 -> out
177+
178+
// Dump pre ECO state
179+
if (debug_) {
180+
dumpVerilogAndOdb(test_name + "_pre_eco");
181+
}
182+
183+
odb::dbDatabase::beginEco(block_);
184+
185+
// Pre sanity check
186+
sta_->updateTiming(true);
187+
// Do not call checkAxioms() because there is an undriven buffer.
188+
189+
//----------------------------------------------------
190+
// Remove buffer
191+
//----------------------------------------------------
192+
auto insts = std::make_unique<sta::InstanceSeq>();
193+
odb::dbInst* buf_inst = block_->findInst("buf1");
194+
ASSERT_NE(buf_inst, nullptr);
195+
sta::Instance* sta_buf = db_network_->dbToSta(buf_inst);
196+
insts->emplace_back(sta_buf);
197+
resizer_.removeBuffers(*insts);
198+
199+
// Post sanity check
200+
sta_->updateTiming(true);
201+
// Do not call checkAxioms() because there is an undriven buffer.
202+
203+
// Write verilog and check the content after buffer removal
204+
const std::string after_vlog_path = test_name + "_after.v";
205+
sta::writeVerilog(after_vlog_path.c_str(), true, false, {}, sta_->network());
206+
207+
std::ifstream file_after(after_vlog_path);
208+
std::string content_after((std::istreambuf_iterator<char>(file_after)),
209+
std::istreambuf_iterator<char>());
210+
211+
// Netlist after buffer removal:
212+
// in -> mod_inst/mod_in -> assign -> mod_inst/mod_out -> out
213+
const std::string expected_after_vlog = R"(module top (clk,
214+
in,
215+
out);
216+
input clk;
217+
input in;
218+
output out;
219+
220+
wire n1;
221+
222+
BUF_X1 buf1 (.Z(n1));
223+
BUF_X1 buf2 (.A(n1),
224+
.Z(out));
225+
endmodule
226+
)";
227+
228+
EXPECT_EQ(content_after, expected_after_vlog);
229+
230+
odb::dbDatabase::undoEco(block_);
231+
232+
// Dump undo ECO state
233+
if (debug_) {
234+
dumpVerilogAndOdb(test_name + "_undo_eco");
235+
}
236+
237+
// Clean up
238+
removeFile(after_vlog_path);
239+
}
240+
33241
TEST_F(BufRemTest3, RemoveBufferCase6)
34242
{
35243
std::string test_name = "TestBufferRemoval3_6";

‎src/rsz/test/cpp/TestBufferRemoval3_7.v‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
// Copyright (c) 2024, The OpenROAD Authors
3+
4+
module top (
5+
input clk,
6+
input in,
7+
output out
8+
);
9+
wire n1;
10+
11+
BUF_X1 buf1 (.Z(n1));
12+
BUF_X1 buf2 (.A(n1), .Z(out));
13+
endmodule

‎src/rsz/test/cpp/TestBufferRemoval3_8.v‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
// Copyright (c) 2024, The OpenROAD Authors
3+
4+
module top (
5+
input clk,
6+
input in,
7+
output out
8+
);
9+
10+
BUF_X1 buf1 (.Z(out));
11+
endmodule

‎src/rsz/test/cpp/TestBufferRemoval3_9.v‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
// Copyright (c) 2024, The OpenROAD Authors
3+
4+
module top (
5+
input clk,
6+
input in,
7+
output out
8+
);
9+
wire n1;
10+
11+
BUF_X1 buf1 (.A(n1), .Z(out));
12+
endmodule

0 commit comments

Comments
 (0)