-
Notifications
You must be signed in to change notification settings - Fork 741
gpl: cover bterms after callbacks processing #7711
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
gpl: cover bterms after callbacks processing #7711
Conversation
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
|
@jeffng-or FYI, you mentioned this issue today I believe. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
I just triggered the updateRules in the ORFS PR for metrics: The-OpenROAD-Project/OpenROAD-flow-scripts#3293 OR CI here has an error for bazel. |
eder-matheus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good, but I wonder if the debugPrints in else statements should be turned into an error message or not. Is there any scenario where the gPins wouldn't be neither a bterm or iterm?
It would not be expected. I just left it to be sure. The callbacks from set_debug_level has other similar checks like this, only for checking unexpected things. |
Callback modifications introduce pointer invalidation to gpl c++ vectors, these invalidations are addressed with
NesterovBaseCommon::fixPointers()which restores references to vector elements. However, it was missing logic to account for dbBTerm objects. As a result, instances connected to bTerms ended up with invalid pin values.The observed effect was that instances connected to bTerms (such as TIE cells with a single pin) had no influence on HPWL and moved solely based on the density gradient, drifting away from their associated bTerm connection.
The issue was resolved with the current branch by including the bTerms loop.
I ran a test-CI at test-gpl-callback-cover-bterms and we are currently having a GRT failure during 5_2 with sky130hd/microwatt: