grt/cugr: implement support for guides restoration#10944
Conversation
…e parity Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
…emand) Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
…ate/OpenROAD into pr4-cugr-restore-guides-v2
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
…ate/OpenROAD into pr4-cugr-restore-guides-v2
|
@codex review |
![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 incremental routing and journal-restored routing capabilities for CUGR, aligning its behavior with FastRoute. Key changes include rebuilding routing trees from ODB guides, updating access point mapping to prevent boundary pin drift, and adding a demand consistency verification mechanism. The code review identified several critical issues: potential out-of-bounds array accesses in buildTreeFromRoute due to missing layer bounds checks, unsafe in-place modification of set elements when merging co-located access points in GridGraph.cpp, and an initialization bug in verifyDemandConsistency where min_base is set to 0.0, rendering the minimum tracking ineffective.
| const int init_layer = segment.init_layer - 1; | ||
| const int final_layer = segment.final_layer - 1; |
There was a problem hiding this comment.
In buildTreeFromRoute, init_layer and final_layer are computed by subtracting 1 from segment.init_layer and segment.final_layer. If these layers are invalid or out of bounds (e.g., < 0 or >= grid_graph_->getNumLayers()), subsequent calls like grid_graph_->getLayerDirection(init_layer) or constructing GRPoint will result in out-of-bounds array accesses or undefined behavior. Adding a bounds check here prevents potential crashes.
const int init_layer = segment.init_layer - 1;
const int final_layer = segment.final_layer - 1;
if (init_layer < 0 || init_layer >= grid_graph_->getNumLayers() ||
final_layer < 0 || final_layer >= grid_graph_->getNumLayers()) {
return nullptr;
}|
|
||
| constexpr double tol = 1e-6; | ||
| double max_diff = 0.0; | ||
| double min_base = 0.0; |
There was a problem hiding this comment.
In verifyDemandConsistency, min_base is initialized to 0.0. Since demand values are non-negative, std::min(min_base, base[l][x][y]) will always evaluate to 0.0, making the minimum tracking ineffective. Initializing min_base to std::numeric_limits<double>::max() ensures the actual minimum base demand is correctly captured.
double min_base = std::numeric_limits<double>::max();
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ 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: Eder Monteiro <emrmonteiro@precisioninno.com>
…l route Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
1 participant
Summary
Completes CUGR's incremental global-routing path to move it closer to FastRoute parity and fixes a pin-access mapping bug that broke guide restoration on congested designs.
Four commits:
the FastRoute incremental path.
Type of Change
Impact
The primary fix corrects how CUGR maps DRT access points to gcells. It previously used a fixed-pitch integer division (
x / (die_width / grid_size)) that drifts off the real gridlines as coordinates grow, placing the router's pin terminalsone gcell off for pins past the drift crossover; it also dropped the access-point layer interval when a pin shared a gcell with APs on another layer. Because
selectAccessPointsshort-circuits to these odb APs whenever DRT pin access ran first (always, in a normal flow), the router's terminals disagreed with every consumer that needs true DRT-AP coverage — guide restoration and the resizer's BufferedNet construction — while staying invisible to the normal flow (DRT redoeslocal pin access). The fix maps APs with the same
rangeSearchCellsgridline search used everywhere else and merges the layer interval for co-located APs.Behavior changes (all CUGR-only; CUGR is opt-in via
-use_cugr/GLOBAL_ROUTE_USE_CUGR, so the default FastRoute flow is unaffected):repair_designno longer crashes with RSZ-0074 (0 → clean).set_debug_level GRT verify_demand 1.Verification
./etc/Build.sh).