DRT: Fix track assignment spacing cost#10503
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 fixes a bug in FlexTA_assign.cpp by moving the instantiation of box2 inside the loop, ensuring it is reset for each track location. It also introduces a new unit test two_parallel_no_coupling and helper methods in the test fixture to verify this behavior. The review feedback suggests a performance optimization in the hot loop of modMinSpacingCostPlanar to directly calculate the vertical distance dy and avoid expensive geometric transformations and box-to-box distance calculations.
| frCoord dx, dy; | ||
| auto& trackLocs = getTrackLocs(lNum); | ||
| auto& workerRegionQuery = getWorkerRegionQuery(); | ||
| for (int i = idx1; i <= idx2; i++) { | ||
| auto trackLoc = trackLocs[i]; | ||
| odb::Rect box2(-halfwidth2, -halfwidth2, halfwidth2, halfwidth2); | ||
| odb::dbTransform xform(odb::Point(boxLeft, trackLoc)); | ||
| xform.apply(box2); | ||
| box2boxDistSquare(box1, box2, dx, dy); |
There was a problem hiding this comment.
Since box1 and box2 always overlap in the X direction (both are aligned around boxLeft), the horizontal distance dx is always 0. Therefore, we can completely avoid constructing box2, creating and applying odb::dbTransform, and calling box2boxDistSquare inside this hot loop. Instead, we can compute the vertical distance dy directly using the interval boundaries of box1 (boxLow, boxHigh) and box2 (trackLoc - halfwidth2, trackLoc + halfwidth2). This simplifies the code and improves track assignment performance.
auto& trackLocs = getTrackLocs(lNum);
auto& workerRegionQuery = getWorkerRegionQuery();
for (int i = idx1; i <= idx2; i++) {
auto trackLoc = trackLocs[i];
frCoord dy = 0;
if (trackLoc - halfwidth2 > boxHigh) {
dy = trackLoc - halfwidth2 - boxHigh;
} else if (boxLow > trackLoc + halfwidth2) {
dy = boxLow - (trackLoc + halfwidth2);
}
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Signed-off-by: osamahammad21 <osama@precisioninno.com>
3 participants
Summary
After implementing the unit tests for Track assignment in DRT, a bug was discovered in Track assignment costing that led to incorrectly dismiss costing tracks in the disallowed spacing vicinity of the assigned track.
Type of Change
Impact
Number of designs: 63
"------------------------------"
Failing designs
"------------------------------"
asap7 mock-alu (base)
Flow reached last stage.
Found 1 metrics failures.
[ERROR] finish__timing__setup__tns fail test: -18422.2 >= -18400
asap7 swerv_wrapper (base)
Flow reached last stage.
Found 1 metrics failures.
[ERROR] finish__timing__setup__tns fail test: -47674.7 >= -47000
nangate45 tinyRocket (base)
Flow reached last stage.
Found 1 metrics failures.
[ERROR] finish__timing__setup__tns fail test: -46.3923 >= -46
sky130hd microwatt (base)
Flow reached last stage.
Found 2 metrics failures.
[ERROR] detailedroute__antenna__violating__nets fail test: 2 <= 1
[ERROR] detailedroute__antenna_diodes_count fail test: 1429 <= 1384
sky130hd riscv32i (base)
Flow reached last stage.
Found 1 metrics failures.
[ERROR] finish__timing__setup__tns fail test: -167.293 >= -167
Verification
./etc/Build.sh).