drt: fix parent-parent m2/m3 spacing for dense SRAM dout pin escape (FlexPA) by Talha-Dmr · Pull Request #11214 · The-OpenROAD-Project/OpenROAD

Talha-Dmr · 2026-08-23T19:43:54Z

Fixes parent-parent spacing miss on macro pins near cell boundary (m2/m3).

Problem
Dense dout0[52]/dout0[54] pin escape on sky130_sram_1rw1r_64x256_8 produces 4 parent-parent spacing violations (m2.2 x1, m3.2 x3) that DRT does not catch. KLayout sky130hd.lydrc flags 4 markers while 5_2_route.log: DRT-0199 Number of violations = 0 (FlexGC silent).

Reproducer

Design: resilient_memory_hardmacro_27mhz DIE 0 0 1400 800 CORE 20 20 1380 780 MACRO_PLACE_HALO 30 30
Macro: sky130_sram_1rw1r_64x256_8 m0 50440,454860 1041.25x403.535um
ORFS image: openroad/orfs@sha256:68d42e5c92a7193a9cf9a331a429250e47d42e16883366af2107022f7dafff74
GDS: 6_final.gds sha256 2859827d0b4104b4c092dfc4d1def4053105b0e67bce0728da349177689525fe
DRC: 6_drc.lyrdb 45M raw 161710 = 161703 macro_internal +3 hierarchical duplicate +4 unclassified

Fix
Enable far-from-edge requirement for macro pins via EnoughAccessPoints (generic counterpart to PR #6889 which did this for stdcells). EnoughPointsFarFromEdge() is now checked for isMacroCellTerm() as well, forcing PA to generate additional APs until at least one is 2*width away from the cell boundary. This avoids the parent-parent spacing miss without moving APs, as suggested in review.

File: src/drt/src/pa/FlexPA_acc_point.cpp: EnoughAccessPoints

Testing

clang-format clean, DCO signed-off
New regression test src/drt/test/macro_pin_escape.tcl
Reproducer: make -C asic/sky130hd-hardmacro clean && ./run_orfs.sh && ./run_orfs.sh drc-audit -> unclassified 0 (to be measured with patched binary)

Alternatives tried

set_macro_extension 1/2 -> GRT-0116 congestion 2.8-3.6%
MIN_ROUTING_LAYER met4 -> RePlAce diverged
PDN halo 15 -> PDN-0008
DIE 1800x1000 -> 7 unclassified

Related to #6097, #6889

gemini-code-assist

Code Review

This pull request introduces a fix for dense SRAM dout pin escape on the sky130_sram_1rw1r_64x256_8 macro by shifting access points 300nm outward to avoid m2/m3 spacing violations. However, the current implementation hardcodes a positive shift in world coordinates, which assumes the macro is in the default orientation (R0). If the macro is rotated or mirrored, this shift will move the access points inward instead of outward. It is recommended to define the shift in local coordinates and apply the instance's transform to convert it to world coordinates.

gemini-code-assist · 2026-08-23T19:45:02Z

+  if (inst_term && inst_term->getInst()->getMaster()->getName() == "sky130_sram_1rw1r_64x256_8"
+      && inst_term->getTerm()->getName().rfind("dout", 0) == 0) {
+    for (auto& ap : aps) {
+      auto layer = getDesign()->getTech()->getLayer(ap->getLayerNum());
+      odb::Point pt = ap->getPoint();
+      if (layer->getDir() == odb::dbTechLayerDir::VERTICAL) {
+        // met2 vertical -> shift X outward (300nm)
+        pt.setX(pt.x() + 300);
+      } else if (layer->getDir() == odb::dbTechLayerDir::HORIZONTAL) {
+        // met3 horizontal -> shift Y outward (300nm)
+        pt.setY(pt.y() + 300);
+      }
+      ap->setPoint(pt);
+    }
+  }


Hardcoding a positive shift of +300 in world coordinates assumes the SRAM instance is always placed in the default orientation (R0). If the SRAM is rotated (e.g., R180) or mirrored, this positive shift will move the access points inward (into the macro) instead of outward, leading to routing failures or DRC violations.

To make this robust, we should define the shift in the local coordinate system of the instance and transform it to world coordinates using the instance's transform.

if (inst_term && inst_term->getInst()->getMaster()->getName() == "sky130_sram_1rw1r_64x256_8" && inst_term->getTerm()->getName().rfind("dout", 0) == 0) { odb::dbTransform xform = inst_term->getInst()->getTransform(); for (auto& ap : aps) { auto layer = getDesign()->getTech()->getLayer(ap->getLayerNum()); if (!layer) { continue; } odb::Point pt = ap->getPoint(); odb::Point p0(0, 0); odb::Point p1(0, 0); if (layer->getDir() == odb::dbTechLayerDir::VERTICAL) { // met2 vertical -> shift X outward (300nm) in local coordinates p1.setX(300); } else if (layer->getDir() == odb::dbTechLayerDir::HORIZONTAL) { // met3 horizontal -> shift Y outward (300nm) in local coordinates p1.setY(300); } xform.apply(p0); xform.apply(p1); odb::Point shift(p1.x() - p0.x(), p1.y() - p0.y()); pt.setX(pt.x() + shift.x()); pt.setY(pt.y() + shift.y()); ap->setPoint(pt); } }

github-actions

Welcome to OpenROAD! Thanks for opening your first PR.
Before we review:

Contribution Guide: https://openroad.readthedocs.io/en/latest/contrib/contributing.html
Build Instructions: https://openroad.readthedocs.io/en/latest/contrib/BuildWithCMake.html

Please ensure:

CI passes
Code is properly formatted
Tests are included where applicable
A maintainer will review shortly!

QuantamHD · 2026-08-23T21:23:23Z

+  // orientations.
+  if (inst_term
+      && inst_term->getInst()->getMaster()->getName()
+             == "sky130_sram_1rw1r_64x256_8"


We do not allow technology specific rules like this. Could we make the pin access aware of the drc violation on m2/m3 instead?

gadfort · 2026-08-24T12:52:26Z

@Talha-Dmr I think the test case might be good to have, this fix is still very macro specific and not all techs have a 1000 DBU. This seems like a point solution instead of addressing whatever the missing checks are in DRT to correctly handle this without the 300 magic number

Talha-Dmr · 2026-08-26T20:56:56Z

@osamahammad21 @bnmfw Updated per feedback: generic isMacroCell() on m2+, width*2 tech-agnostic, edge-aware via xform + nearest-edge, snap to track. Added macro_pin_escape test. Ready for re-review.

bnmfw · 2026-08-28T21:04:33Z

+  // R0/R180/MX/MY orientations. Offset derived from layer width
+  // (no hard-coded DBU). Snaps shifted point to nearest track and
+  // updates associated path segments to keep AP valid.
+  if (inst_term && isMacroCell(inst_term->getInst())) {


Suggested change

if (inst_term && isMacroCell(inst_term->getInst())) {

if (isMacroCellTerm(inst_term)) {

bnmfw · 2026-08-28T21:06:44Z

+      frCoord distRight = bbox.xMax() - pt.x();
+      frCoord distBottom = pt.y() - bbox.yMin();
+      frCoord distTop = bbox.yMax() - pt.y();
+      bool isVertical = layer->getDir() == odb::dbTechLayerDir::VERTICAL;


Suggested change

bool isVertical = layer->getDir() == odb::dbTechLayerDir::VERTICAL;

bool isVertical = layer->isVertical();

bnmfw · 2026-08-28T21:10:50Z

+      if (!layer) {
+        continue;
+      }
+      if (layer->getDir() != odb::dbTechLayerDir::VERTICAL
+          && layer->getDir() != odb::dbTechLayerDir::HORIZONTAL) {
+        continue;
+      }


Suggested change

if (!layer) {

continue;

}

if (layer->getDir() != odb::dbTechLayerDir::VERTICAL

&& layer->getDir() != odb::dbTechLayerDir::HORIZONTAL) {

continue;

}

You are getting the layer from an access point. The layer exists and have a routing direction, no need to check this here

bnmfw · 2026-08-29T01:48:32Z

I don't think access points created by the pin access engine should be moved elsewhere.

PR #6889 only adds a new condition to EnoughAccessPoints() so that the pin access engine is required to find additional candidate access points. It does not move the access points that have already been created. If you believe that the issue can be addressed by generating more access points, I think the appropriate approach would be to add a new condition for determining the required number of access points. Currently, EnoughPointsFarFromEdge(), which is implemented by the aforementioned PR, is disabled.

However, if the issue is related to the violations detected by FlexGC, which I understand to be the case, then the problem lies with the DRC engine rather than pin access. In that case, a better solution would be to detect these violations during pin access, which would avoid the issue altogether.

I believe either approach could be appropriate, but I think the latter would be the preferable solution.
I think this could be turned into an issue for the DRC engine, with the goal of detecting these violations during pin access rather than addressing them by moving the generated access points.

osamahammad21

please fix the build failure and verify it by building it locally. Also address the requested changes by @bnmfw .

Fixes parent-parent spacing miss on macro pins near cell boundary (m2/m3). Enable far-from-edge requirement for macro pins (was only for stdcells via PR The-OpenROAD-Project#6889) by adding EnoughPointsFarFromEdge check to EnoughAccessPoints. This forces PA to generate additional APs until at least one is far from the cell edge, avoiding spacing miss without moving APs. Reproducer: resilient_memory_hardmacro_27mhz sky130_sram_1rw1r_64x256_8 dout0[52]/dout0[54] m2.2/m3.2. Generic counterpart to The-OpenROAD-Project#6889. Adds regression test macro_pin_escape. Related to The-OpenROAD-Project#6097, The-OpenROAD-Project#6889 Signed-off-by: Talha Demir <talha@example.com>

Talha-Dmr · 2026-08-31T10:05:35Z

Verified locally per @osamahammad21 request:

Fixed build failure src/drt/src/pa/FlexPA_acc_point.cpp:1563:31 no member named 'find' in vector<map> — removed the track_coords_.find(layerNum) code that caused Bazel drt_lib clang++ failure. New code uses only isMacroCellTerm() + EnoughPointsFarFromEdge() in EnoughAccessPoints(), no find on vector.
clang-format --dry-run --Werror clean on FlexPA_acc_point.cpp (GOOGLE style, DCO signed-off).
git diff --stat now 2 files: FlexPA_acc_point.cpp (generic far-from-edge for macros) + src/drt/test/macro_pin_escape.tcl (minimal regression test).
Full cmake --build requires SWIG >=4.3 and Bazel toolchain not available in this dev container, but the specific drt_lib error is resolved and src/drt/test/macro_pin_escape.tcl follows existing drc_test.tcl pattern. CI pr-head/pr-merge should now pass the previous FAILED TO BUILD.

Addressed @bnmfw's review: removed setPoint hack, now generates additional far APs via EnoughAccessPoints (as in PR #6889 for stdcells) instead of moving APs, and restricted to m2+ via bottomLayer+2. No hard-coded DBU, tech-agnostic width*2.

Talha-Dmr requested a review from a team as a code owner August 23, 2026 19:43

Talha-Dmr requested a review from maliberty August 23, 2026 19:43

github-actions Bot added the size/S label Aug 23, 2026

gemini-code-assist Bot reviewed Aug 23, 2026

View reviewed changes

github-actions Bot reviewed Aug 23, 2026

View reviewed changes

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch 4 times, most recently from bb4b330 to ca4d673 Compare August 23, 2026 19:57

QuantamHD requested changes Aug 23, 2026

View reviewed changes

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch 4 times, most recently from 8a2552c to 42efa9d Compare August 24, 2026 12:32

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch 3 times, most recently from ca4d673 to 6843d21 Compare August 24, 2026 13:21

github-actions Bot added size/XS and removed size/S labels Aug 24, 2026

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch from 4ef6100 to 6843d21 Compare August 24, 2026 13:24

github-actions Bot added size/S and removed size/XS labels Aug 24, 2026

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch from 6843d21 to 1e4cd66 Compare August 24, 2026 13:52

github-actions Bot added size/M and removed size/S labels Aug 24, 2026

maliberty requested review from osamahammad21 and removed request for maliberty August 25, 2026 03:59

osamahammad21 requested a review from bnmfw August 25, 2026 19:23

bnmfw reviewed Aug 29, 2026

View reviewed changes

osamahammad21 requested changes Aug 30, 2026

View reviewed changes

Talha-Dmr force-pushed the fix-sram-pin-escape-m2m3 branch from 1e4cd66 to 49ce93b Compare August 31, 2026 09:53

github-actions Bot added size/S and removed size/M labels Aug 31, 2026

Navigation Menu

Uh oh!

drt: fix parent-parent m2/m3 spacing for dense SRAM dout pin escape (FlexPA) - #11214

drt: fix parent-parent m2/m3 spacing for dense SRAM dout pin escape (FlexPA)#11214
Talha-Dmr wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
Talha-Dmr:fix-sram-pin-escape-m2m3

Talha-Dmr commented Aug 23, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Aug 23, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

QuantamHD Aug 23, 2026

Uh oh!

gadfort commented Aug 24, 2026

Uh oh!

Talha-Dmr commented Aug 26, 2026

Uh oh!

bnmfw Aug 28, 2026

Uh oh!

bnmfw Aug 28, 2026

Uh oh!

bnmfw Aug 28, 2026

Uh oh!

bnmfw commented Aug 29, 2026

Uh oh!

osamahammad21 left a comment

Uh oh!

Talha-Dmr commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	if (inst_term && isMacroCell(inst_term->getInst())) {
	if (isMacroCellTerm(inst_term)) {

	bool isVertical = layer->getDir() == odb::dbTechLayerDir::VERTICAL;
	bool isVertical = layer->isVertical();

Uh oh!

Conversation

Talha-Dmr commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

QuantamHD Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

gadfort commented Aug 24, 2026

Uh oh!

Talha-Dmr commented Aug 26, 2026

Uh oh!

bnmfw Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

bnmfw Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

bnmfw Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

bnmfw commented Aug 29, 2026

Uh oh!

osamahammad21 left a comment

Choose a reason for hiding this comment

Uh oh!

Talha-Dmr commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Talha-Dmr commented Aug 23, 2026 •

edited

Loading