dpl: remove universal cell height assumption#10684
Conversation
… for all instances, instead fetch instance height from db to define pixel grid height relative to instance Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…pixels Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the NegotiationLegalizer to support hybrid-row designs with non-uniform row heights. It removes the uniform row_height_ member variable and instead utilizes dpl_grid APIs (such as gridSnapDownY, gridYToDbu, and gridHeight) to handle coordinate conversions and height calculations. Additionally, it adds logging to report cell height distributions. There are no review comments, and I have no further feedback to provide.
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
@osamahammad21 @eder-matheus I hope this one to be more straight forward. |
|
@gudeh Does it require a secure-ci? We have some private designs with negotiation-based on, right? |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 673cb79826
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| FenceRect r; | ||
| r.xlo = (box->xMin() - die_xlo_) / site_width_; | ||
| r.ylo = (box->yMin() - die_ylo_) / row_height_; | ||
| r.ylo = dpl_grid->gridSnapDownY(DbuY{box->yMin() - die_ylo_}).v; |
There was a problem hiding this comment.
When a fence or group region's lower Y edge falls inside a variable-height grid interval, gridSnapDownY rounds the lower bound down to the previous row, so FenceRegion::contains() can accept a cell whose bottom is below the DEF region. The regular DPL region path snaps contained rectangles inward via gridWithin()/gridEndY() for the lower edge; negotiation should do the same here (and in the init-time region cache) or hybrid/fragmented-row designs with non-row-aligned fences can be legalized outside their assigned region.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This seems relevant. At Grid.cpp we originally use .ylo = gridEndY(rect.yl), instead of gridSnapDownY() at:
OpenROAD/src/dpl/src/infrastructure/Grid.cpp
Lines 607 to 613 in c6efacc
its no-op: http://secure-ci:8080/blue/organizations/jenkins/SB/detail/secure-dpl-cell-height/1/pipeline Maybe it is because we have a single-height row setup with multi-row instances. |
make sure we do not include one grid row outside the region range Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
2 participants
Summary
Remove the assumption that every instance shares a single universal
row_height_in the negotiation legalizer. Add cell-height distribution reporting.Replace every
row_height_usage with the DPL grid's own helpers, which already account for per-row heights:In the negotiation legalizer, log the distribution in grid row-counts (info message DPL-392/393). Easy to see at a glance whether a design is single- or multi-height.
Type of Change
Impact
Some private designs with hybrid rows or multi height rows would present issues because of this assumption.
On uniform-height designs this is no-op. The grid helpers reduce to the same arithmetic the old code did with
row_height_.Verification
./etc/Build.sh).Related Issues
This is one of multiple PRs splitting up PR #10226 to make Negotiation the default algorithm at DPL.