Skip to content

Conversation

@openroad-ci
Copy link
Collaborator

No description provided.

Signed-off-by: Matt Liberty <[email protected]>
Signed-off-by: Matt Liberty <[email protected]>
Signed-off-by: Matt Liberty <[email protected]>
The transform usage in drt is non-standard and should be cleaned up.

Signed-off-by: Matt Liberty <[email protected]>
No value to the intermediate classes and removes dead methods.

Signed-off-by: Matt Liberty <[email protected]>
Signed-off-by: Matt Liberty <[email protected]>
Signed-off-by: Matt Liberty <[email protected]>
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.

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


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

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>


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

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>

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants