Repository navigation
odb: avoid moved port names on buffered driver nets - #11815
jinchuntan wants to merge 1 commit into
Conversation
Check every moved BTerm when renaming flat nets and modnets during buffer insertion. Keep the first BTerm as the new net name. Cover both original port names and partial port moves in flat and hierarchical designs, including Verilog export and re-import. Fixes The-OpenROAD-Project#11789 Signed-off-by: Nigel Tan <nigeltanjc@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request updates dbInsertBuffer::createNewFlatNet to iterate over all connected terms and rename colliding flat nets or modnets for any port moving to the new net, rather than just checking a single port. It also adds a comprehensive unit test suite (TestInsertBuffer) to verify this behavior. The review feedback points out a potential null pointer dereference in the new loop over connected_terms if any element is nullptr, and suggests adding a null check to prevent a crash.
There was a problem hiding this comment.
If connected_terms contains a nullptr (which is possible since load_pins can contain null pointers as handled in validateLoadPinsAndFindLCA), calling obj->getObjectType() will result in a null pointer dereference and crash the application.
Adding a null check prevents this potential crash.
for (dbObject* obj : connected_terms) {
if (obj == nullptr) {
continue;
}
if (obj->getObjectType() != dbBTermObj) {
continue;
}
Summary
Check every moved port when renaming the original flat net and its modnets during buffer insertion. The first port still supplies the new net's name.
Type of Change
Impact
When two output ports share a net, moving both behind a buffer no longer leaves the driver's net named after the second port. Writing and reading the Verilog preserves separate buffer input and output nets.
Verification
Eight cases cover both original port names, flat and hierarchical linking, and moving one or both ports. Each case exports Verilog and reads it back to check connectivity. Two cases reproduce the short on unchanged code and pass with the fix.
All 54 tests across these targets pass:
bazelisk test --test_output=errors \ //src/odb/test/cpp:TestInsertBuffer \ //src/odb/test/cpp:OdbGTests \ //src/odb/test/cpp:TestModuleLocal validation used the pinned LLVM dependency with archive ownership restoration disabled for this container. Remote caching and build telemetry were disabled. The full OpenROAD application and ORFS flow were not built.
Related Issues
Fixes #11789