Skip to content

mpl: keep snapped macros inside the core - #11275

Closed
oharboe wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
oharboe:mpl-snap-in-core
Closed

mpl: keep snapped macros inside the core#11275
oharboe wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
oharboe:mpl-snap-in-core

Conversation

@oharboe

@oharboe oharboe commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

The Snapper picks the track (or manufacturing-grid) position closest to
the pin center, rounding up when in doubt. For a macro at the top or
right core edge this moves the macro bounding box slightly past the core
boundary. The residue is invisible to the flow that produced it, but
place_macro rejects such a position with MPL-0034 — so the file written
by -write_macro_placement does not round-trip: the placer's own winner
cannot be re-injected via MACRO_PLACEMENT_TCL.

This PR constrains the track-alignment search to positions that keep the
macro inside the core, and nudges the macro inward by whole
manufacturing-grid steps when no aligned position qualifies (new warning
MPL-0078). Fixed macros are untouched, as before.

Note on goldens: the regenerated goldens of boundary_push1,
fixed_macros2, halos5 and orientation_improve1/3 all contained
placer-produced macros poking past the core boundary (verified by
comparing each macro bbox against dbBlock::getCoreArea()); their macros
now land inside the core, track-aligned one pitch inward.

Tests: two new TestSnapper unit cases (manufacturing-grid and
track-aligned containment) and macro_placement_round_trip1, which checks
write_macro_placement -> place_macro lands every macro exactly where the
placer left it.

🤖 Generated with Claude Code

The Snapper picked the track (or manufacturing-grid) position closest
to the pin center, rounding up when in doubt. For a macro at the top or
right core edge this moved the macro bounding box slightly past the
core boundary. The residue is invisible to the flow that produced it,
but place_macro rejects such a position with MPL-0034, so the file
written by -write_macro_placement did not round-trip: the placer's own
winner could not be re-injected with MACRO_PLACEMENT_TCL.

Constrain the track-alignment search to positions that keep the macro
inside the core, and nudge the macro inward by whole manufacturing-grid
steps when no aligned position qualifies (new warning MPL-0078). Fixed
macros are untouched as before.

The regenerated goldens of boundary_push1, fixed_macros2, halos5 and
orientation_improve1/3 all contained placer-produced macros poking past
the core boundary; their macros now land inside the core, track-aligned
one pitch inward.

The new macro_placement_round_trip1 test checks the write_macro_placement
-> place_macro round-trip lands every macro exactly where the placer
left it.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe requested a review from a team as a code owner August 31, 2026 01:01
@oharboe
oharboe requested a review from AcKoucher August 31, 2026 01:01

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that macro placement remains within the core boundary during snapping operations by introducing validation and adjustment helpers (macroStaysInCore and nudgeMacroIntoCore). It also updates affected test outputs and adds new unit and integration tests. The review feedback suggests adding defensive checks in nudgeMacroIntoCore to handle degenerate cores and prevent potential division-by-zero crashes.

Comment thread src/mpl/src/snapper.cpp
Comment on lines +345 to +348
const odb::Rect core = inst_->getBlock()->getCoreArea();
const odb::Rect bbox = inst_->getBBox()->getBox();
const int manufacturing_grid
= inst_->getDb()->getTech()->getManufacturingGrid();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To prevent potential runtime issues, we should add defensive checks in nudgeMacroIntoCore:

  1. Degenerate Core Check: If the core area is degenerate (e.g., before rows are created), we should return early to avoid unnecessary or incorrect shifting, matching the behavior in macroStaysInCore.
  2. Division by Zero Guard: If manufacturing_grid is 0 or negative (which can happen with incomplete technology data), the rounding-up division will cause a division-by-zero crash. Defaulting to 1 (the minimum database unit) avoids this.
Suggested change
const odb::Rect core = inst_->getBlock()->getCoreArea();
const odb::Rect bbox = inst_->getBBox()->getBox();
const int manufacturing_grid
= inst_->getDb()->getTech()->getManufacturingGrid();
const odb::Rect core = inst_->getBlock()->getCoreArea();
if (core.dx() == 0 || core.dy() == 0) {
return;
}
const odb::Rect bbox = inst_->getBBox()->getBox();
int manufacturing_grid
= inst_->getDb()->getTech()->getManufacturingGrid();
if (manufacturing_grid <= 0) {
manufacturing_grid = 1;
}

@oharboe

oharboe commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Closing per our downstream plan: carrying this as a patch in bazel-orfs while mpl churns, will re-upstream once it settles. Tracked with a minimal reproducer in #11278 (which also notes the place_macro self-overlap observation).

@oharboe oharboe closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant