dpl: modify default algorithm to negotiation legalizer#10226
Conversation
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…alizer Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
after reviewing the updated tests I realized there is something wrong with upf_aes, which is composed of regions. |
![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 transitions the default detailed placement legalizer to the NegotiationLegalizer and introduces a -use_old_diamond flag for the legacy engine. Key feedback includes fixing a logic error in Opendp.cpp where the legacy flag incorrectly persists across sessions due to bitwise OR assignment. Refactoring is also suggested to eliminate duplicated code for illegal cell identification in NegotiationLegalizerPass.cpp. Furthermore, the reviewer noted that several test expectation files were updated to include diff failure messages, which undermines the verification process; these should be corrected by updating the underlying golden reference files instead.
| std::vector<int> illegal_cells; | ||
| for (int idx : active) { | ||
| if (!cells_[idx].fixed && !isCellLegal(idx)) { | ||
| illegal_cells.push_back(idx); | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic to identify illegal cells for diamond recovery is duplicated between Phase 1 (lines 95-100) and Phase 2 (lines 156-161). This logic should be refactored into a helper method to improve maintainability and reduce code duplication. Additionally, there is a minor inconsistency in the warning messages between the two phases: Phase 1 includes a newline and the word "old" (line 105), while Phase 2 does not (line 165). It would be better to make these messages consistent for easier log parsing.
References
- To improve maintainability and reduce redundancy, extract duplicated logic into a helper function to avoid code duplication.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Is the current option in use by ORFS? |
We need an update on ORFS too for removing the |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…alizer Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
secure-CI presented 3 fatals! At least one of them negotiation legalizer is not converging to a solution! I will investigate further. |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
stop every iteration on iterative mode, include stop on final state, introduce iterative_jump to stop at desired iteration Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
sky130hd/aes - nangate45/swerv - sky130hd/gcd - second GRT/DPL call negotation did not converge. Although, it starts with 98.4% utilization. |
|
I also noticed all instances in upf_aes test are being solved with diamond search. The diamond search is used as a fallback when we can't find a solution for a few instances. GPL finishes with some region-less instances (top-level) on top of other regions, in the following image they are the ones in green: |
|
Even with current default behavior we get this awkward legalization because region-less instances are placed in other regions by gpl |
…oves in grey, track current-iter movers Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
anyway, the situation with UPF is unrelated to the negotiation legalizer. |
…sites, print table similar to gpl with controled number of lines, less verbose, rename overflow to violations Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
gudeh
force-pushed
the
dpl-negotiation-default
branch
from
dba4e07 to
b61a343
Compare
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>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
gudeh
force-pushed
the
dpl-negotiation-default
branch
from
6cc7d33 to
edb85ab
Compare
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…dex' into dpl-negotiation-default
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
make sure the window fits properly with valid rows, Y range is given by min between: (cell height * constantY), versus max y displacement X range is given by min between: max(constantX, cell width), versus max x displacement Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
include new debug messa for debuging window range Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Match the canonical check: bottom-row site type only, plus checkRowPowerCompatible for multi-row masters. Also extend the search-window draw rect up to max_anchor + cell.height so it encloses the cell at the topmostcandidate row. Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…cosistent with legacy rail checking at checkRowPowerCompatible Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
2 participants
Summary
Modify DPL default algorithm from diamond search to negotiation legalizer. Switch the toggle from
-use_negotiationto-use_old_diamond.Type of Change
Impact
All DPL calls will use the negotiation legalizer instead of the former diamond search.
Verification
./etc/Build.sh).