Skip to content

Thin drt db#9288

Open
openroad-ci wants to merge 14 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:thin-drt-db
Open

Thin drt db#9288
openroad-ci wants to merge 14 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:thin-drt-db

Conversation

@openroad-ci
Copy link
Collaborator

@openroad-ci openroad-ci commented Jan 19, 2026

No description provided.

@maliberty
thin the fr data
f2f1e33 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
odb: rm frInst::name_ and get it from dbInst::name()
d133791 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: rm setTransform from frRef & frVia
cd616db 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: rm getOrient from frRef & frVia
54fa177 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: rm getTransform from frRef & frVia
382fa42 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: clang-format
5303170 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: rm frRef
3ffd2cf 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: rm useless comments in src/db/obj
89cc87a 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: check db_inst is non-null in frInst
a2501de 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: fix typo frInstBlockage::setIndexeInOwner -> setIndexInOwner
0cce8ae 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
Remove transform from frInst and take it from dbInst
04fda29 
The transform usage in drt is non-standard and should be cleaned up.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: change frInst's base class frPinFig -> frBlockObject
facaad7 
No value to the intermediate classes and removes dead methods.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: clang-format
6926aaf 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty
drt: clang-tidy
3fe6b0c 
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
gemini-code-assist[bot]
gemini-code-assist bot reviewed Jan 19, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a significant and well-executed refactoring to "thin" the drt database objects. Key changes include:

  • Removing duplicated state (like name and transform) from frInst and making it a lightweight wrapper around odb::dbInst.
  • Simplifying the class hierarchy by removing the frRef intermediate class. frInst now inherits directly from frBlockObject, and frVia from frPinFig.
  • Consistently updating the codebase to reflect these changes, including constructors, accessors, and related logic in various modules like pin access, IO, and GC.
  • Cleaning up code by removing unnecessary comments and fixing minor typos.

These changes effectively reduce memory usage and code complexity, improving maintainability. The refactoring appears to be correct and consistently applied. I have not found any issues in this pull request.

github-actions[bot]
github-actions bot reviewed Jan 19, 2026
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/drt/src/db/obj/frInst.cpp

#include "db/obj/frBlock.h"
#include <algorithm>
#include <memory>
Copy link
Contributor

@github-actions github-actions bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: included header algorithm is not used directly [misc-include-cleaner]

Suggested change
#include <memory>
#include <memory>

src/drt/src/db/obj/frInst.cpp

void frInst::addInstTerm(std::unique_ptr<frInstTerm> in)
{
instTerms_.push_back(std::move(in));
Copy link
Contributor

@github-actions github-actions bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "std::move" is directly included [misc-include-cleaner]

src/drt/src/db/obj/frInst.cpp:8:

+ #include <utility>

src/drt/src/db/obj/frVia.cpp
void frVia::serialize(Archive& ar, const unsigned int version)
{
(ar) & boost::serialization::base_object<frRef>(*this);
(ar) & boost::serialization::base_object<frPinFig>(*this);
Copy link
Contributor

@github-actions github-actions bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "boost::serialization::base_object" is directly included [misc-include-cleaner]

src/drt/src/db/obj/frVia.cpp:4:

+ #include <boost/serialization/base_object.hpp>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@maliberty maliberty

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@openroad-ci @maliberty