GPL: add support to TD with multiple regions#9389
GPL: add support to TD with multiple regions#9389LucasYuki wants to merge 12 commits intoThe-OpenROAD-Project:masterfrom
Conversation
![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 introduces important fixes and features for region-based placement in GPL. The changes to handle cell creation and destruction within specific regions are well-implemented, particularly the refactoring in NesterovBase to correctly update all placer instances after a cell is destroyed and replaced. The bug fix in DPL for the dummy_cell_ is also a good catch.
I've found one logic issue in the new createCbkGCell function in nesterovPlace.cpp where a warning is logged incorrectly. My review includes a specific comment with a suggested fix for this.
Overall, this is a solid set of changes that improves the robustness and capabilities of the placer.
|
clang-tidy review says "All clean, LGTM! 👍" |
2 similar comments
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
is this no-op for current designs without regions? |
|
we should have secure-CI for this branch PR |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@gudeh, I ran a secure-CI and it didn't change any current designs without regions. |
LucasYuki
force-pushed
the
gpl-TD-region
branch
from
9dd9b7a to
705966c
Compare
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: LucasYuki <46359888+LucasYuki@users.noreply.github.com>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
LucasYuki
force-pushed
the
gpl-TD-region
branch
from
705966c to
68494cc
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| deepIterativePause("pause after legalGridPt() inside shiftMove(), cell " | ||
| + target_cell->name()); | ||
|
|
||
| deepIterativePause("pause after legalGridPt() inside shiftMove(), cell " | ||
| + target_cell->name()); |
| set_layer_rc -via via2 -resistance 3.368786E-3 | ||
| set_layer_rc -via via3 -resistance 0.376635E-3 | ||
| set_layer_rc -via via4 -resistance 0.00580E-3 | ||
| set_layer_rc -via via4 -resistance 0.00580E-3 No newline at end of file |
There was a problem hiding this comment.
No reason to remove the last newline. Do you have an editor setting issue?
| global_placement -skip_initial_place -density uniform -routability_driven -timing_driven | ||
|
|
||
| detailed_placement | ||
| detailed_placement -max_displacement 1000 |
There was a problem hiding this comment.
Why is such a large limit needed?
3 participants
Part of #8838.
Changes:
dbRegion(inDbInstCreate).NesterovBaseobject, but the cells are stored in theNesterovBaseCommonNesterovBaseCommon, we replace the deleted cell with the last cell in the list. After this, we need to update the index of this cell in allNesterovBases.dummy_cell_was not set as fixed, causing a segmentation fault in some cases.inDbInstCreate(dbInst* inst, dbRegion* region)callback.inDbInstCreate(dbInst* inst)is necessary.This PR doesn't assign regions to buffers inserted by the RSZ.
Currently, all buffers are placed in the top region, but support for inserting instances in a dbRegion was added.