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}
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! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
gudeh
force-pushed
the
dpl-negotiation-default
branch
from
b831c65 to
41ed990
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
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! 👍" |
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
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).