grt: implement CUGR net merge mirroring FastRoute path by sparsh-karna · Pull Request #10899 · The-OpenROAD-Project/OpenROAD

sparsh-karna · 2026-07-13T17:17:08Z

Summary

When GlobalRouter::mergeNetsRouting() fires for the CUGR path, this PR enables attempting to stitch the two existing route segments at the former buffer pin position (connectCUGRRouting) rather than ripping up and rerouting the survivor from scratch.

Key changes:

GlobalRouter::connectCUGRRouting(preserved, removed): Mirrors connectRouting() but uses cugr_->hasAvailableResources() instead of fastroute_->hasAvailableResources() for the capacity check. Reuses the existing geometry helpers findBufferPinPostions, findTopLayerOverPosition, and createConnectionForPositions, which are FastRoute-agnostic.
GlobalRouter::mergeNetsRouting() (CUGR branch): If stitching succeeds, calls cugr_->mergeNet() to transfer tree ownership and save guides. On failure, falls back to addDirtyNet() so the survivor is rerouted incrementally (matching the previous safe behavior).
CUGR::mergeNet(preserved, removed): Attaches the removed net's GRTreeNode subtree as a child of the preserved net's routing tree so getRoutes() emits all wire segments. Inserts the removed net into merged_nets_ so removeNet() knows not to decrement GridGraph demand for it (the wires are still physically there, just owned by the preserved net).
CUGR::removeNet() (merged-net guard): Skips removeTreeUsage() for nets that were transferred via mergeNet, preventing double-decrement of GridGraph congestion numbers.
CUGR::hasAvailableResources(layer, tile_x, tile_y): A thin wrapper over GridGraph::getEdge().getResource() >= 1.0, equivalent to FastRouteCore::hasAvailableResources().

Type of Change

New feature

Impact

Improves QoR (Quality of Results) and runtime during incremental routing by preserving existing routing topology during buffer removal ECOs when using CUGR, matching FastRoute's behavior.

Verification

I have verified that the local build succeeds (./etc/Build.sh).
I have run the relevant tests and they pass. (All 143 GRT regression tests pass.)
My code follows the repository's formatting guidelines.
I have included tests to prevent regressions.
I have signed my commits (DCO).

Related Issues

None.

When GlobalRouter::mergeNetsRouting() fires for the CUGR path, attempt to stitch the two existing route segments at the former buffer pin position (connectCUGRRouting) rather than ripping up and rerouting the survivor from scratch. Key changes: GlobalRouter::connectCUGRRouting(preserved, removed) Mirrors connectRouting() but uses cugr_->hasAvailableResources() instead of fastroute_->hasAvailableResources() for the capacity check. Reuses the existing geometry helpers findBufferPinPostions, findTopLayerOverPosition, and createConnectionForPositions which are FastRoute-agnostic. GlobalRouter::mergeNetsRouting() -- CUGR branch If stitching succeeds, call cugr_->mergeNet() to transfer tree ownership and save guides. On failure, fall back to addDirtyNet so the survivor is rerouted incrementally (matching the previous safe behaviour). CUGR::mergeNet(preserved, removed) Attaches the removed net's GRTreeNode subtree as a child of the preserved net's routing tree so getRoutes() emits all wire segments. Inserts the removed net into merged_nets_ so removeNet() knows not to decrement GridGraph demand for it (the wires are still there, now owned by the preserved net). CUGR::removeNet() -- merged-net guard Skips removeTreeUsage() for nets that were transferred via mergeNet, preventing double-decrement of GridGraph congestion numbers. CUGR::hasAvailableResources(layer, tile_x, tile_y) Thin wrapper over GridGraph::getEdge().getResource() >= 1.0, equivalent to FastRouteCore::hasAvailableResources. All 143 GRT regression tests pass. Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

gemini-code-assist

Code Review

This pull request implements support for merging nets in CUGR rather than always falling back to a full dirty-net reroute. It introduces connectCUGRRouting to stitch two CUGR routes together at the former buffer pin position and perform capacity checks against the CUGR GridGraph, and updates the CUGR class to handle routing tree ownership transfer during net merges. Feedback was provided to add an early return check if either route is empty to prevent a potential crash when finding the top layer over a position.

eder-matheus · 2026-07-13T17:22:39Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98e1a41ad8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

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: Sparsh Karna <sparsh2005karna@gmail.com>

eder-matheus

Please include unit tests in this PR. You can use the existing remove_buffer tests to create the new tests for the CUGR path.

Remove the standalone connectCUGRRouting() function and fold the CUGR-specific capacity check (against GridGraph) into the existing connectRouting() function via a use_cugr_ branch. A new optional connection_out parameter receives the stitching segments when the caller needs to update the CUGR routing tree. The mergeNetsRouting() function now uses a single connectRouting() call. Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

Consolidate mergeNetsRouting() to use a single connectRouting() logic block for both FastRoute and CUGR paths, applying CUGR-specific capacity checks inside connectRouting() and performing the final CUGR tree merge conditionally after routing is connected. Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

Add error logging with ID 705 to prevent negative layer indices from failing silently in CUGR::hasAvailableResources(). Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

Rename nodeA/B, target_layerA/B, target_xA/B, target_yA/B, and findPath to snake_case equivalent to comply with OpenROAD coding guidelines. Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

eder-matheus · 2026-07-15T16:49:41Z

+    } else {
+      // After failing to connect the routing, the survivor net still has
+      // uncovered pins and needs to be re-routed
+      net1->setDirtyNet(true);


Why is this FastRoute exclusive? Shouldn't this be marked on the CUGR path as well?

You're right, it shouldn't be disjointed. The reason for the split is that setDirtyNet(true) toggles a boolean flag on FastRoute's internal GRNet struct (which CUGR ignores), whereas CUGR requires us to explicitly enqueue the net via addDirtyNet() . However, we can definitely clean this up. We should be able to unify this fallback block to handle both the FastRoute internal state and the CUGR queue consistently without this verbose branching. I'll update it to be cleaner.

eder-matheus · 2026-07-15T16:50:05Z

+      net1->setIsMergedNet(true);
+      net1->setMergedNet(db_net2);
+      net1->setDirtyNet(false);
+      net2->setIsMergedNet(true);
+      net2->setMergedNet(db_net1);


Should this be exclusive to FastRoute path?

Yes, this should be exclusive to the FastRoute path. Those methods manipulate the GRNet struct, which is FastRoute's internal data model. CUGR has its own tracking for merged nets (which gets updated inside cugr_->mergeNet() ). Setting these GRNet flags when using CUGR wouldn't break anything, but it would be dead code since CUGR doesn't read them. Restricting it to the FastRoute path makes it much clearer which internal data model is actively driving the routing.

I don't think this is correct. net1 and net2 are GlobalRouter::Net* objects, and these functions are only touching this representation. This "merged nets" related flags are important for other steps of the incremental flow to not reroute the nets by accident.

eder-matheus · 2026-07-15T16:52:58Z

+  if (connectRouting(db_net1, db_net2, &connection_segs)) {
    saveGuides({db_net1});
+    if (use_cugr_) {
+      cugr_->mergeNet(db_net1, db_net2, connection_segs);


Why don't you call this inside the connectRouting function? This would remove the need for this extra argument for the connection_segs, and would make all the merge/connection between nets contained to a single function.

I'll move it there.

sparsh-karna requested a review from a team as a code owner July 13, 2026 17:17

sparsh-karna requested a review from jfgava July 13, 2026 17:17

github-actions Bot added the size/M label Jul 13, 2026

gemini-code-assist Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/grt/src/GlobalRouter.cpp Outdated

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/grt/src/cugr/src/CUGR.cpp Outdated

eder-matheus self-requested a review July 13, 2026 17:29

sparsh-karna added 3 commits July 13, 2026 17:30

grt: add early return check for empty routes in CUGR net merge

be400db

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

grt: preserve connection geometry in CUGR tree on merge

941b89b

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

grt: run clang-format to fix format check violations

b2232b6

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

sparsh-karna force-pushed the cugr-merge-net branch from 74660a9 to b2232b6 Compare July 13, 2026 17:47

eder-matheus requested changes Jul 14, 2026

View reviewed changes

Comment thread src/grt/src/GlobalRouter.cpp Outdated

Comment thread src/grt/src/GlobalRouter.cpp Outdated

Comment thread src/grt/src/cugr/src/CUGR.cpp

Comment thread src/grt/src/cugr/src/CUGR.cpp Outdated

sparsh-karna added 4 commits July 15, 2026 15:14

grt: error on invalid layer index in CUGR hasAvailableResources

fd16683

Add error logging with ID 705 to prevent negative layer indices from failing silently in CUGR::hasAvailableResources(). Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

grt: fix snake_case variable names in CUGR::mergeNet

1397874

Rename nodeA/B, target_layerA/B, target_xA/B, target_yA/B, and findPath to snake_case equivalent to comply with OpenROAD coding guidelines. Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>

eder-matheus requested changes Jul 15, 2026

View reviewed changes

Uh oh!

Conversation

sparsh-karna commented Jul 13, 2026

Summary

Type of Change

Impact

Verification

Related Issues

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

eder-matheus commented Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

eder-matheus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eder-matheus Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sparsh-karna Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

eder-matheus Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sparsh-karna Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

eder-matheus Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

eder-matheus Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sparsh-karna Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants