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

mpl: use only the moveable macros during random cluster resizing in SA #6697

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AcKoucher
Copy link
Contributor

@AcKoucher AcKoucher commented Feb 12, 2025

This should impact the results generated by #6649.
I expect MPL results to change significantly with this bug fix.

Context

The io_constraints2 regression test started to fail with the changes in #6689 which didn't seem to make sense as the changes there should only cause impact on designs with meaningful connections and this regression test is a fit-only test (no nets).

Cause of Failure

The problem is that, inside the cluster placement (Soft) SA, the function responsible for randomly resizing one cluster has a type of move that instead of relying on the location/shape of just the moveable SoftMacros - that is, the SoftMacros from the sequence pair - it uses all the SoftMacros including the fixed terminals, which interferes/messes up the computation of the new width/height for the target cluster.

In the image (just a dummy example), it is as if the fixed terminal position/shape could impact in a possible resizing move of A and B. (Cyan outline is the current level in which SA in working on).

Reminder: the SACore object receives a vector of Soft/Hard Macros and just some of them are the actual sequence pair, the others might be io clusters - for root level - or fixed terminals.

I.e., the position/shape/amount of fixed terminals should not impact on the resizing move.

Changes Here

Ensure that we only use the macros of the sequence pair to generate the new width/height for the target cluster during a cluster resizing move in cluster placement SA.

With the changes here, the regression test result gets consistent.

…m cluster resize inside SASoft

Signed-off-by: Arthur Koucher <[email protected]>
Copy link
Contributor

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

@AcKoucher
Copy link
Contributor Author

Running Secure-CI

@AcKoucher AcKoucher requested a review from maliberty February 12, 2025 15:54
Copy link
Member

@maliberty maliberty left a comment

Choose a reason for hiding this comment

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

Looks good; waiting on CI

@AcKoucher
Copy link
Contributor Author

This blew up the worst slack / TNS and WL in one private design. I'm investigating.

@rovinski
Copy link
Collaborator

@AcKoucher thank you for the great writeup. Explaining the issue like this is invaluable for understanding changes and bug fixes.

@AcKoucher AcKoucher added ant Antenna Checker and removed ant Antenna Checker labels Feb 17, 2025
@AcKoucher
Copy link
Contributor Author

AcKoucher commented Feb 18, 2025

The investigation showed that, for cases in which the area used for macro placement is substantially smaller than the core, GPL (which uses the entire core) might end up placing std cells very far from each other causing timing paths to get very long and loopy.

Restricting GPL to the same are used by MPL makes it output a better placement timing wise than it would having the whole core available (even without using timing-driven).

@gudeh is investigating more deeply.

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