[MahmoudNedalSubLaban]

Summary

Fixes #11697.

The region query returns a whole via when its combined bounding box intersects a GC worker's extBox. This can import a landing outside the context while omitting its connecting wire, creating a false spacing marker.

The production fix checks each transformed metal/cut figure in initRouteObj. Intersecting figures, including touching and crossing figures, are retained whole. The marker collector and repairPDNVias implementation are unchanged.

Review revision

The tests now exercise the existing GCFixture worker directly, setting extBox/drcBox and checking its markers. The full-router/parser harness, extra dst dependency, private friend and header-export changes are removed.

The complete PR changes only FlexGC_init.cpp and gcTest.cpp. No CMake or BUILD changes remain.

Coverage includes both metal landings and transposed axes, the connected-metal reproducer, genuine and legal gaps, and exact translated metal/cut imports at outside, touching, crossing and contained positions.

Validation

These local results are for fixture revision fc2a628280, before the upstream synchronization below.

• Fresh Release build of OpenROAD and GC/TA/Watermark test binaries.
• All 153 enabled GC tests pass in three final shuffled runs.
• Removing only the three guards produces the same 34 failures in each of three shuffled runs; the other 119 cases pass. Restoring the guards restores all passes.
• All 236 registered DRT/PDN/GC/TA CTest entries pass three executions each.
• All five WatermarkCost tests pass three executions.
• Formatting and whitespace checks pass.
• Local Bazel GC and TA targets pass three fresh runs each with --config=ci and test-result caching disabled.

One pre-existing disabled GC case is unchanged on the tested PR base. Counts include unit-test launchers and are not counts of independent physical designs. Hosted CI and maintainer review are still required; this is a reproduced checker-correctness fix, not a routing/PPA improvement claim.

Upstream synchronization

Commit ee2cfd7b8c incorporates upstream base 1bcacf76a0, the exact base used by successful Jenkins merge build 25. Its complete source tree is identical to that tested merge (619dd5bc64).

This updates the old sv-lang dependency configuration that failed archive-checksum verification in the Docker stages of head build 6. The DRT fix and fixture tests are unchanged, and no checksum validation was bypassed. New commit-specific CI is pending.

[gemini-code-assist]

Code Review

This pull request updates TritonRoute's marker retrieval to filter out markers that do not intersect with the worker's DRC box, matching the ownership filter used when committing markers. It adds a getter for the DRC box in FlexGCWorker and introduces several integration tests to verify boundary behavior. Feedback suggests optimizing the intersection check order in TritonRoute.cpp to leverage short-circuit evaluation and avoid the overhead of calling the non-inline getDrcBox() function for non-intersecting markers.

[MahmoudNedalSubLaban]

Fixed the Bazel test registration in 1cd3cc7. All five new tests had reached pass, but the default check_log=True then looked for nonexistent .ok files. They now use the existing pass/fail convention; other DRT tests retain log comparison.

Locally, the old flags reproduce all five failures and the corrected flags pass all five through test/bazel_test.sh, using the existing binary. The unpatched binary still fails the defect assertion. Buildifier and whitespace checks pass. C++ and fixtures are unchanged; the full CI rerun is pending.

[osamahammad21]

Please use the gcTest.cpp for testing any GC functionality. The tcl tests are for testing the tcl APIs and interface only and they should be kept to minimum

[osamahammad21]

How would that happen? I think the gcWorker shouldn't have markers that don't intersect with its DRC box.

[MahmoudNedalSubLaban]

Addressed in 000496e: the five cases now use gcTest.cpp; their Tcl scripts/fixtures are removed.

I reproduced the out-of-region marker in connected_metal. initNetsFromDesign() queries extBox, and initRouteObj() imports all figures of a selected via. Here, extBox.xMin = 27400: the M3 landing reaches 27430, but the connecting M2 wire ends at 27360 and is omitted. The worker reports M2 box (27220,3160)-(27360,3230), outside its drcBox.xMin = 28900 (DBU).

addMarker() deduplicates without clipping. Routing applies the ownership filter later in FlexDRWorker::endAddMarkers(); this patch applies it to the checker collector too.

All ten new cases pass. Removing the filter fails only the two connected-metal cases; the real-gap and boundary controls remain passing.

[MahmoudNedalSubLaban]

Thanks for checking. I reproduced this in gcTest.cpp: a worker imports a via but misses the connecting wire outside its extBox, creating a false spacing marker outside drcBox. addMarker() deduplicates markers without checking that boundary. The routing path applies this filter later; the patch adds the same check to the fresh-checker collector.

[osamahammad21]

I see, so the real problem seems to be in the DRC engine: a worker can report a violation outside its check region because it doesn’t have the full context there. Still, I’m not comfortable silently dropping those markers. My main concern is that getDRCMarkers isn’t only used for check_drc. repairPDNVias uses it too, so this change could alter physical output. I’m also not sure every real violation is guaranteed to be caught by the GC worker that owns its region.

In general, GC relies on later routing iterations to clean up incorrectly reported violations.

[MahmoudNedalSubLaban]

Thanks, I agree that filtering the shared collector was too broad. It also left the question of whether another worker would catch a genuine violation unanswered.

I traced the reproducer to via geometry imported outside the worker’s context. The revised fix addresses that import and restores getDRCMarkers to its original behavior. It no longer discards reported markers based on worker ownership.

The connected-metal case now agrees with the full-context check, while the genuine-gap controls still report violations. I also exercised repairPDNVias through the C++ tests and checked the surviving vias and array cuts, rather than only checking marker counts. Its implementation is unchanged in this revision.

This addresses the reproduced input inconsistency without relying on a later routing iteration to remove the false marker.

[osamahammad21]

I see, so the real problem seems to be in the DRC engine: a worker can report a violation outside its check region because it doesn’t have the full context there. Still, I’m not comfortable silently dropping those markers. My main concern is that getDRCMarkers isn’t only used for check_drc. repairPDNVias uses it too, so this change could alter physical output. I’m also not sure every real violation is guaranteed to be caught by the GC worker that owns its region.

In general, GC relies on later routing iterations to clean up incorrectly reported violations.

[MahmoudNedalSubLaban]

Hi @osamahammad21, I pushed the revised fix in 7afc9dc.

I removed the filter from getDRCMarkers and moved the correction to initRouteObj. The reproducer imported a via landing outside extBox while omitting its connecting wire, creating a false spacing violation. The revised code checks each transformed via figure against the context; touching and crossing figures remain whole. It no longer discards markers based on worker ownership.

As requested, all new GC coverage is in gcTest.cpp; no Tcl regressions were added. The tests cover genuine violations, full-context comparisons, shifted boundaries, multiple threads, and actual repairPDNVias outputs. The separate PDN repair issue is kept out of this PR.

Locally, 455 GC tests and 236 DRT/PDN CTest entries passed three runs. Bazel GC and TA tests also passed three runs with the CI warning checks enabled.

CI is still pending, and the GitHub Actions runs show action_required. Could you take another look once the checks have run?

[osamahammad21]

Please don’t pull dst (and a full TritonRoute) into gcTest. This is a unit test for FlexGCWorker, and the fix lives in initRouteObj, so it can be tested on the existing fixture worker by setting extBox/drcBox around a via that sits on the boundary and checking the markers

[MahmoudNedalSubLaban]

Thank you for the guidance @osamahammad21 . I replaced the full-router harness with direct fixture-worker tests using extBox/drcBox and marker assertions, and removed the extra dependency and header changes. The production fix is unchanged, and local regression tests pass.

[MahmoudNedalSubLaban]

Head build #8 passed. Merge build #27 failed in the Ubuntu 26.04 and Debian 12 Docker stages because downloading m4-1.4.18 from the GNU/kernel mirrors timed out before compilation. Could you please check CI mirror/cache access and rerun the merge job? The DRT fix and fixture tests are unchanged. Thank you.