Skip to content

Comments

rsz: Fix and clean up size move preliminaries#8237

Merged
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:rsz-sizing-check
Sep 4, 2025
Merged

rsz: Fix and clean up size move preliminaries#8237
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:rsz-sizing-check

Conversation

@openroad-ci
Copy link
Collaborator

Make sure we don't attempt to size an instance which isn't a standard cell. Also clean up the clone move condition. This condition looks to have been introduced in commit 533c037 as a workaround for undo limitations (mistakenly with flipped logic) and carried over to other places.

Make sure we don't attempt to size an instance which isn't a standard
cell. Also clean up the clone move condition. This condition looks to
have been introduced in commit 533c037 as a workaround for undo
limitations (mistakenly with flipped logic) and carried over to other
places.

Signed-off-by: Martin Povišer <povik@cutebit.org>
@povik
Copy link
Contributor

povik commented Sep 4, 2025

FYI @mguthaus @precisionmoon regarding the clone move cleanup

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2025

clang-tidy review says "All clean, LGTM! 👍"

@povik povik requested a review from maliberty September 4, 2025 20:45
// We always size the cloned gates for some reason, but it would be good if we
// also down-sized here instead since we might want smaller original.
if (!resizer_->dontTouch(drvr)
|| resizer_->clone_move_->hasPendingMoves(drvr)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the clone_move_->hasPendingMoves check should only matter for instances with dont touch. For that reason I'm expecting none or few metric changes. I can run a secure CI if we don't want to chance it.

Comment on lines +126 to +127
if (resizer_->dontTouch(load_inst)
|| !resizer_->isLogicStdCell(load_inst)) {
Copy link
Member

Choose a reason for hiding this comment

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

I see similar checks in various places. Perhaps you could do a refactor similar to okToBufferNet like okToResizeInstance ?

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to do this here or in another?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather do this elsewhere. We could fold isLogicStdCell into dontTouch as I'm not sure there's any transformation we want the resizer to apply to macros.

@maliberty maliberty merged commit b970fb4 into The-OpenROAD-Project:master Sep 4, 2025
11 checks passed
@maliberty maliberty deleted the rsz-sizing-check branch September 4, 2025 22:08
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