From fc1be7bc7858182ac0bdb2c2c17cb5486832cfa8 Mon Sep 17 00:00:00 2001 From: Jaehyun Kim Date: Fri, 6 Mar 2026 17:59:58 +0900 Subject: [PATCH 1/2] odb: fix createNewFlatNet flat net and modnet name collision with BTerm When remove_buffers merges two port-named nets, a BTerm can end up on a net with a different port's name. Additionally, the modnet retains the original port name (stale). createNewFlatNet now always names the new net after the BTerm and renames any colliding flat net or modnet first. This fixes spurious assign statements and multi-driver errors in VerilogWriter output that caused Kepler LEC failures in hierarchical designs (e.g., bp_fe_top). Co-Authored-By: Claude Opus 4.6 Signed-off-by: Jaehyun Kim --- src/odb/src/db/dbInsertBuffer.cpp | 63 ++++++++++------- src/rsz/test/buffer_ports10.defok | 4 +- src/rsz/test/buffer_ports8.defok | 4 +- src/rsz/test/buffer_ports9.defok | 4 +- src/rsz/test/cpp/TestInsertBuffer.cpp | 70 +++++++++++++++++++ .../TestInsertBuffer_BeforeLoad_Case1_post.v | 4 +- ...TestInsertBuffer_BeforeLoads_Case10_post.v | 4 +- ...TestInsertBuffer_BeforeLoads_Case12_post.v | 6 +- ...TestInsertBuffer_BeforeLoads_Case13_post.v | 8 +-- ...TestInsertBuffer_BeforeLoads_Case33_post.v | 21 ++++++ .../TestInsertBuffer_BeforeLoads_Case33_pre.v | 26 +++++++ .../TestInsertBuffer_BeforeLoads_Case3_post.v | 4 +- .../TestInsertBuffer_BeforeLoads_Case9_post.v | 4 +- 13 files changed, 168 insertions(+), 54 deletions(-) create mode 100644 src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_post.v create mode 100644 src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_pre.v diff --git a/src/odb/src/db/dbInsertBuffer.cpp b/src/odb/src/db/dbInsertBuffer.cpp index c66163346da..f42d2eb5506 100644 --- a/src/odb/src/db/dbInsertBuffer.cpp +++ b/src/odb/src/db/dbInsertBuffer.cpp @@ -447,46 +447,55 @@ dbNet* dbInsertBuffer::createNewFlatNet( // Algorithm: // - The new net will drive the connected_terms, and the original net will be // connected to the buffer terminal. - // - If the original net name matches a connected BTerm, rename the original - // net to a unique name and assign the BTerm's name to the new net. - // This ensures that the net name matches the port name for Verilog - // compatibility. + // - If connected_terms contains a BTerm, the new net is always named after + // the BTerm to ensure Verilog compatibility. If the original net or its + // modnet has the same name, they are renamed first to avoid collision. + // This handles the case where remove_buffers merges two port-named nets, + // leaving a BTerm on a net with a different port's name. std::string new_net_name = (new_net_base_name_) ? new_net_base_name_ : "net"; dbNameUniquifyType new_net_uniquify = uniquify_; - // Check if the net name conflicts with any port name + // Find the first BTerm in connected_terms (if any) to use its port name + // as the new net name for Verilog compatibility. + dbBTerm* bterm = nullptr; for (dbObject* obj : connected_terms) { - if (obj->getObjectType() != dbBTermObj) { - continue; + if (obj->getObjectType() == dbBTermObj) { + bterm = static_cast(obj); + break; } + } - dbBTerm* bterm = static_cast(obj); + if (bterm) { std::string_view bterm_name{bterm->getConstName()}; - if (bterm_name == block_->getBaseName(net_->getConstName())) { - // The original net names uses the BTerm name. - // Rename this net if its name is the same as a port name in loads_pins - std::string new_orig_net_name = block_->makeNewNetName( + + // Helper: generate a unique name to avoid collision with the port name. + auto make_avoidance_name = [&]() { + return block_->makeNewNetName( target_module_ ? target_module_->getModInst() : nullptr, new_net_name.c_str(), dbNameUniquifyType::ALWAYS); - net_->rename(new_orig_net_name.c_str()); - - // Rename the mod net name connected to the load pin if it is the - // same as the port name - dbModNet* load_mnet = bterm->getModNet(); - if (load_mnet) { - std::string_view mnet_name{load_mnet->getConstName()}; - if (mnet_name == bterm_name) { - load_mnet->rename(new_orig_net_name.c_str()); - } - } + }; - // New net name should be the port name - new_net_name = bterm_name; - new_net_uniquify = dbNameUniquifyType::IF_NEEDED; - break; + // Rename the original flat net if it has the same name as the port. + std::string avoidance_name; + if (bterm_name == block_->getBaseName(net_->getConstName())) { + avoidance_name = make_avoidance_name(); + net_->rename(avoidance_name.c_str()); + } + + // Rename the modnet if it has the same name as the port. + dbModNet* load_mnet = bterm->getModNet(); + if (load_mnet + && std::string_view{load_mnet->getConstName()} == bterm_name) { + if (avoidance_name.empty()) { + avoidance_name = make_avoidance_name(); + } + load_mnet->rename(avoidance_name.c_str()); } + + new_net_name = bterm_name; + new_net_uniquify = dbNameUniquifyType::IF_NEEDED; } // Create a new net diff --git a/src/rsz/test/buffer_ports10.defok b/src/rsz/test/buffer_ports10.defok index 709e6134c64..8786382491c 100644 --- a/src/rsz/test/buffer_ports10.defok +++ b/src/rsz/test/buffer_ports10.defok @@ -100,7 +100,7 @@ PINS 4 ; + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 100510 199930 ) N ; - - out2 + NET net4 + DIRECTION OUTPUT + USE SIGNAL + - out2 + NET out2 + DIRECTION OUTPUT + USE SIGNAL + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 98990 199930 ) N ; @@ -111,7 +111,7 @@ NETS 7 ; - net1 ( input1 Z ) ( r1 CK ) + USE SIGNAL ; - net2 ( input2 Z ) ( r1 D ) + USE SIGNAL ; - net3 ( output4 A ) ( output3 A ) ( r1 Q ) + USE SIGNAL ; - - net4 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; - out1 ( PIN out1 ) ( output3 Z ) + USE SIGNAL ; + - out2 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; END NETS END DESIGN diff --git a/src/rsz/test/buffer_ports8.defok b/src/rsz/test/buffer_ports8.defok index 709e6134c64..8786382491c 100644 --- a/src/rsz/test/buffer_ports8.defok +++ b/src/rsz/test/buffer_ports8.defok @@ -100,7 +100,7 @@ PINS 4 ; + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 100510 199930 ) N ; - - out2 + NET net4 + DIRECTION OUTPUT + USE SIGNAL + - out2 + NET out2 + DIRECTION OUTPUT + USE SIGNAL + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 98990 199930 ) N ; @@ -111,7 +111,7 @@ NETS 7 ; - net1 ( input1 Z ) ( r1 CK ) + USE SIGNAL ; - net2 ( input2 Z ) ( r1 D ) + USE SIGNAL ; - net3 ( output4 A ) ( output3 A ) ( r1 Q ) + USE SIGNAL ; - - net4 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; - out1 ( PIN out1 ) ( output3 Z ) + USE SIGNAL ; + - out2 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; END NETS END DESIGN diff --git a/src/rsz/test/buffer_ports9.defok b/src/rsz/test/buffer_ports9.defok index 3a597ae19b8..2237d067d31 100644 --- a/src/rsz/test/buffer_ports9.defok +++ b/src/rsz/test/buffer_ports9.defok @@ -100,7 +100,7 @@ PINS 4 ; + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 100510 199930 ) N ; - - out2 + NET net4 + DIRECTION OUTPUT + USE SIGNAL + - out2 + NET out2 + DIRECTION OUTPUT + USE SIGNAL + PORT + LAYER metal2 ( -70 -70 ) ( 70 70 ) + PLACED ( 98990 199930 ) N ; @@ -111,7 +111,7 @@ NETS 7 ; - net1 ( input1 Z ) ( r1 CK ) + USE SIGNAL ; - net2 ( input2 Z ) ( r1 D ) + USE SIGNAL ; - net3 ( output4 A ) ( output3 A ) ( r1 Q ) + USE SIGNAL ; - - net4 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; - out1 ( PIN out1 ) ( output3 Z ) + USE SIGNAL ; + - out2 ( PIN out2 ) ( output4 Z ) + USE SIGNAL ; END NETS END DESIGN diff --git a/src/rsz/test/cpp/TestInsertBuffer.cpp b/src/rsz/test/cpp/TestInsertBuffer.cpp index c79de89af4b..24f46fea104 100644 --- a/src/rsz/test/cpp/TestInsertBuffer.cpp +++ b/src/rsz/test/cpp/TestInsertBuffer.cpp @@ -3199,4 +3199,74 @@ TEST_F(TestInsertBuffer, BeforeLoads_Case32) writeAndCompareVerilogOutputFile(test_name, test_name + "_post.v"); } +// Reproduce the real bug: remove_buffers + insertBufferBeforeLoad +// on a hierarchical design. +// +// Synthesis creates a submodule (MOD0) with two output ports: +// MOD0: LOGIC0_X1 drvr -> Z_a, BUF_X1 cross_buf: Z_a -> Z_b +// At the top level: +// MOD0 mod0 (.Z_a(port_a), .Z_b(port_b)) +// +// The cross_buf connects two port-named nets (port_a and port_b). +// remove_buffers removes it, merging port_b into port_a. +// BTerm port_b ends up on net port_a — name mismatch. +// +// Then insertBufferBeforeLoad inserts a buffer before port_b. +// Without the fix, createNewFlatNet gives the new net a generic name, +// and write_verilog produces: +// MOD0 mod0 (.Z_a(port_a), .Z_b(port_b)); -- submodule drives port_b +// assign port_b = net1; -- assign also drives port_b +// This is a multi-driver on port_b, rejected by Kepler LEC. +TEST_F(TestInsertBuffer, BeforeLoads_Case33) +{ + const auto* test_info = testing::UnitTest::GetInstance()->current_test_info(); + const std::string test_name + = std::string(test_info->test_suite_name()) + "_" + test_info->name(); + + readVerilogAndSetup(test_name + "_pre.v"); + + dbMaster* buf_x4_master = db_->findMaster("BUF_X4"); + ASSERT_NE(buf_x4_master, nullptr); + + // Verify initial state: BTerm port_b matches its net name + dbBTerm* bterm_b = block_->findBTerm("port_b"); + ASSERT_NE(bterm_b, nullptr); + EXPECT_STREQ(bterm_b->getConstName(), bterm_b->getNet()->getConstName()); + + // --- Step 1: remove_buffers (reproduces the real flow) --- + // Removes cross_buf that connects port_a -> port_b. + // Both nets have BTerms, so survivor = input net (port_a). + // BTerm port_b moves to net port_a — name mismatch. + resizer_.removeBuffers({}); + + EXPECT_EQ(block_->findInst("cross_buf"), nullptr) + << "cross_buf should have been removed by remove_buffers"; + EXPECT_STREQ(bterm_b->getNet()->getConstName(), "port_a") + << "After remove_buffers, BTerm port_b should be on net port_a " + "(name mismatch)"; + + // --- Step 2: insertBufferBeforeLoad on port_b (reproduces buffer_ports) --- + dbNet* target_net = bterm_b->getNet(); + dbInst* new_buf = target_net->insertBufferBeforeLoad( + bterm_b, buf_x4_master, nullptr, "output"); + ASSERT_NE(new_buf, nullptr); + + // The buffer output net must be named "port_b" — not a generic name. + // Without the fix, createNewFlatNet produces "net1" because + // bterm_name ("port_b") != net_name ("port_a"), skipping the rename. + // This causes write_verilog to emit a spurious assign statement, + // and in hierarchical designs, creates a multi-driver because the + // submodule port map still references port_b. + dbNet* buf_out_net = new_buf->findITerm("Z")->getNet(); + ASSERT_NE(buf_out_net, nullptr); + EXPECT_EQ(bterm_b->getNet(), buf_out_net); + EXPECT_STREQ(buf_out_net->getConstName(), "port_b") + << "Buffer output net must be named after the BTerm (port_b), " + "not a generic name. Without the fix, this would be 'net1' " + "and write_verilog would emit 'assign port_b = net1;'"; + + // Write verilog and compare + writeAndCompareVerilogOutputFile(test_name, test_name + "_post.v"); +} + } // namespace odb diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoad_Case1_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoad_Case1_post.v index bd3837d3af7..979b5e40222 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoad_Case1_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoad_Case1_post.v @@ -4,19 +4,17 @@ module top (load_output); wire net; wire net1; wire net3; - wire net4; BUF_X4 buf1 (.A(net), .Z(net1)); BUF_X4 buf3 (.A(net), .Z(net3)); BUF_X4 buf4 (.A(net), - .Z(net4)); + .Z(load_output)); LOGIC0_X1 drvr_inst (.Z(net)); BUF_X1 load0_inst (.A(net1)); BUF_X1 load2_inst (.A(net3)); MOD0 mi0 (.A(net)); - assign load_output = net4; endmodule module MOD0 (A); input A; diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case10_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case10_post.v index 4547b38ad5b..a5d461c9686 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case10_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case10_post.v @@ -4,9 +4,7 @@ module top (in, output out; wire n1; - wire net1; BUF_X4 new_buf1 (.A(n1), - .Z(net1)); - assign out = net1; + .Z(out)); endmodule diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case12_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case12_post.v index e5d26aa4b0a..b493e4b3360 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case12_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case12_post.v @@ -2,11 +2,9 @@ module top (out); output out; wire n1; - wire net1; BUF_X1 drvr (.Z(n1)); - BUF_X1 load1 (.A(net1)); + BUF_X1 load1 (.A(out)); BUF_X4 new_buf1 (.A(n1), - .Z(net1)); - assign out = net1; + .Z(out)); endmodule diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case13_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case13_post.v index f3106b32fee..c97594551f7 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case13_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case13_post.v @@ -1,15 +1,13 @@ module top (out); output out; - wire net1; wire n1; BUF_X1 drvr (.Z(n1)); - BUF_X1 load1 (.A(net1)); + BUF_X1 load1 (.A(out)); BUF_X4 new_buf1 (.A(n1), - .Z(net1)); - MOD1 u1 (.A(net1)); - assign out = net1; + .Z(out)); + MOD1 u1 (.A(out)); endmodule module MOD1 (A); input A; diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_post.v new file mode 100644 index 00000000000..cca353f504c --- /dev/null +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_post.v @@ -0,0 +1,21 @@ +module top (port_a, + port_b); + output port_a; + output port_b; + + wire net1; + + MOD0 mod0 (.Z_a(port_a), + .Z_b(net1)); + BUF_X4 output1 (.A(net1), + .Z(port_b)); +endmodule +module MOD0 (Z_a, + Z_b); + output Z_a; + output Z_b; + + + LOGIC0_X1 drvr (.Z(Z_a)); + assign Z_b = Z_a; +endmodule diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_pre.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_pre.v new file mode 100644 index 00000000000..83ea3cb9c1b --- /dev/null +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case33_pre.v @@ -0,0 +1,26 @@ +// Reproduce the real bug: remove_buffers + insertBufferBeforeLoad. +// Hierarchical design where a submodule drives two output ports, +// with a buffer connecting port_a to port_b inside the submodule. +// +// After remove_buffers + insertBufferBeforeLoad (buggy): +// VerilogWriter emits both submodule .Z_b(port_b) AND assign port_b = net1 +// → multi-driver on port_b +module top (port_a, + port_b); + output port_a; + output port_b; + + + MOD0 mod0 (.Z_a(port_a), + .Z_b(port_b)); +endmodule +module MOD0 (Z_a, + Z_b); + output Z_a; + output Z_b; + + + LOGIC0_X1 drvr (.Z(Z_a)); + BUF_X1 cross_buf (.A(Z_a), + .Z(Z_b)); +endmodule diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case3_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case3_post.v index 9a17e452525..7c4e2542131 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case3_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case3_post.v @@ -5,13 +5,11 @@ module top (in, wire n2; wire n1; - wire net2; MOD0 mod0 (.out(n2), .in(n1)); BUF_X4 new_buf22 (.A(n2), - .Z(net2)); - assign out = net2; + .Z(out)); endmodule module MOD0 (out, in); diff --git a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case9_post.v b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case9_post.v index f560785d969..22c555e647f 100644 --- a/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case9_post.v +++ b/src/rsz/test/cpp/TestInsertBuffer_BeforeLoads_Case9_post.v @@ -2,10 +2,8 @@ module top (out); output out; wire n1; - wire net1; BUF_X1 drvr (.Z(n1)); BUF_X4 new_buf1 (.A(n1), - .Z(net1)); - assign out = net1; + .Z(out)); endmodule From 03a2d1434275ade896dd1ff9f0ec58bec790f169 Mon Sep 17 00:00:00 2001 From: Jaehyun Kim Date: Sat, 7 Mar 2026 09:00:01 +0900 Subject: [PATCH 2/2] odb: refactor createNewFlatNet collision logic for readability Refactor the flat net and modnet renaming logic to separate collision detection from the renaming action. This improves readability by determining collisions upfront and handling renames within a single conditional block. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Jaehyun Kim --- src/odb/src/db/dbInsertBuffer.cpp | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/odb/src/db/dbInsertBuffer.cpp b/src/odb/src/db/dbInsertBuffer.cpp index f42d2eb5506..c9e3d931d67 100644 --- a/src/odb/src/db/dbInsertBuffer.cpp +++ b/src/odb/src/db/dbInsertBuffer.cpp @@ -477,21 +477,23 @@ dbNet* dbInsertBuffer::createNewFlatNet( dbNameUniquifyType::ALWAYS); }; - // Rename the original flat net if it has the same name as the port. - std::string avoidance_name; - if (bterm_name == block_->getBaseName(net_->getConstName())) { - avoidance_name = make_avoidance_name(); - net_->rename(avoidance_name.c_str()); - } - - // Rename the modnet if it has the same name as the port. + // Rename the original flat net and/or modnet if they have the same name as + // the port. + const bool flat_net_collides + = (bterm_name == block_->getBaseName(net_->getConstName())); dbModNet* load_mnet = bterm->getModNet(); - if (load_mnet - && std::string_view{load_mnet->getConstName()} == bterm_name) { - if (avoidance_name.empty()) { - avoidance_name = make_avoidance_name(); + const bool mod_net_collides + = (load_mnet + && std::string_view{load_mnet->getConstName()} == bterm_name); + + if (flat_net_collides || mod_net_collides) { + const std::string avoidance_name = make_avoidance_name(); + if (flat_net_collides) { + net_->rename(avoidance_name.c_str()); + } + if (mod_net_collides) { + load_mnet->rename(avoidance_name.c_str()); } - load_mnet->rename(avoidance_name.c_str()); } new_net_name = bterm_name;