dpl: negotiation, introduce regions and fix power rail alignment#10175
dpl: negotiation, introduce regions and fix power rail alignment#10175gudeh wants to merge 11 commits intoThe-OpenROAD-Project:masterfrom
Conversation
fix rail alignment for negotiation, debug: include cell orientation when selected 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>
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 enhances the Negotiation Legalizer by deriving power-rail types from DB row orientations and LEF geometry, and adding support for region-constrained cells via groups. Feedback focuses on performance optimizations to avoid bottlenecks in debug printing and ODB lookups, preventing console flooding from warnings, and correcting logic for cell flippability and orientation visualization.
| if (cell.height == 1) { | ||
| // Consider all single height cells flippable | ||
| cell.flippable = true; |
There was a problem hiding this comment.
Restricting the automatic flippability to only cell.height == 1 excludes multi-row cells with an odd number of rows (e.g., 3-row cells). These cells also have different power rail types at their top and bottom edges, and thus flipping them is a valid way to resolve rail mismatches. The previous logic using % 2 == 1 was more general and correct for rail alignment.
| if (cell.height == 1) { | |
| // Consider all single height cells flippable | |
| cell.flippable = true; | |
| if (cell.height % 2 == 1) { | |
| // Consider all odd height cells flippable |
There was a problem hiding this comment.
I was actually considering to remove this and keeping only the getSymmetryX() which comes right before these lines and not override it.
| odb::dbRegion* odb_region = db_inst->getRegion(); | ||
| if (odb_region == nullptr) { | ||
| auto* grp = db_inst->getGroup(); | ||
| if (grp != nullptr) { | ||
| odb_region = grp->getRegion(); | ||
| } | ||
| } | ||
| struct RegionRectInline | ||
| { | ||
| int xlo, ylo, xhi, yhi; | ||
| }; | ||
| std::vector<RegionRectInline> region_rects_inline; | ||
| if (odb_region != nullptr) { | ||
| for (auto* box : odb_region->getBoundaries()) { | ||
| RegionRectInline r; | ||
| r.xlo = (box->xMin() - die_xlo_) / site_width_; | ||
| r.ylo = (box->yMin() - die_ylo_) / row_height_; | ||
| r.xhi = (box->xMax() - die_xlo_) / site_width_; | ||
| r.yhi = (box->yMax() - die_ylo_) / row_height_; | ||
| region_rects_inline.push_back(r); | ||
| } | ||
| } |
There was a problem hiding this comment.
Accessing ODB to fetch region boundaries and converting them to grid coordinates for every single instance is inefficient, as many instances typically share the same region. This information should be cached in a map (e.g., std::unordered_map<odb::dbRegion*, std::vector<RegionRectInline>>) to avoid redundant processing.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
transform warn into debugPrint Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
Summary
Introduce regions knowledge to the new negotiation legalizer and remove assumptions about power rail alignment.
Type of Change
Impact
Negotiation can legalize based on regions and fixes some issues with rail alignment during legalization.
Verification
./etc/Build.sh).Related Issues
After the change in PR #10167, the negotiation legalizer is failing during check_placement.