Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dpl: Replace Diamond search with a BFS #6693

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

openroad-ci
Copy link
Collaborator

Replaces Diamond Search with a BFS to minimize single cell displacement.

The diamond search finds the closest free site to place a cell, but the distance was measured in sites.
Now the search finds the closest free site measured in dbu.

Since only the displacement of individual cells is optimized, this doesn't mean that the total displacement is always reduced compared to the current diamond search.
There are test cases where the total displacement increases, but in most cases, the total displacement is smaller than before.

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

int& best_dist) const;
PixelPt binSearch(GridX x, const Cell* cell, GridX bin_x, GridY bin_y) const;
PixelPt searchNearestSite(const Cell* cell, GridX x, GridY y) const;
int calcDist(const GridPt p0, const GridPt p1) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'p0' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
int calcDist(const GridPt p0, const GridPt p1) const;
int calcDist(GridPt p0, const GridPt p1) const;

int& best_dist) const;
PixelPt binSearch(GridX x, const Cell* cell, GridX bin_x, GridY bin_y) const;
PixelPt searchNearestSite(const Cell* cell, GridX x, GridY y) const;
int calcDist(const GridPt p0, const GridPt p1) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'p1' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
int calcDist(const GridPt p0, const GridPt p1) const;
int calcDist(const GridPt p0, GridPt p1) const;

PixelPt searchNearestSite(const Cell* cell, GridX x, GridY y) const;
int calcDist(const GridPt p0, const GridPt p1) const;
bool canBePlaced(const Cell* cell,
const GridX bin_x,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'bin_x' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const GridX bin_x,
GridX bin_x,

int calcDist(const GridPt p0, const GridPt p1) const;
bool canBePlaced(const Cell* cell,
const GridX bin_x,
const GridY bin_y) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'bin_y' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const GridY bin_y) const;
GridY bin_y) const;

Signed-off-by: Lucas Yuki Imamura <[email protected]>
Signed-off-by: Lucas Yuki Imamura <[email protected]>
@@ -248,4 +250,15 @@ struct hash<dpl::TypedCoordinate<T>>
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This weakens the type checking. Is it needed?

std::unordered_set<GridPt> inQueuePositions;
GridPt center{x, y};
positionsHeap.push(PQ_entry{0, center});
inQueuePositions.insert(center);
Copy link
Member

Choose a reason for hiding this comment

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

When a point is popped from the heap it is not removed from inQueuePositions. It therefore is misleadingly named as you can be in inQueuePositions but not actually be queued. I think visited would be clearer.

@maliberty
Copy link
Member

Some small comments but generally fine. Waiting on CI and conflict resolution.

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

3 participants