Skip to content

mpl: keep snapped macros inside the core #11901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
naveenvenk17 wants to merge 4 commits into
base: master
Choose a base branch
from
+276 −31
Open

mpl: keep snapped macros inside the core #11901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1d91a8
mpl: keep snapped macros inside the core
naveenvenk17 Oct 21, 2026
267a495
build: keep shared lockfile repair out of feature changes
naveenvenk17 Oct 21, 2026
9cbb408
mpl: reuse the pin offset while filtering snap candidates
naveenvenk17 Oct 21, 2026
a3bc878
ci: rerun checks after the upstream lockfile repair
naveenvenk17 Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 88 additions & 17 deletions src/mpl/src/snapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,41 @@ void Snapper::snap(const odb::dbTechLayerDir& target_direction)
: inst_->getOrigin().y();

if (layers_data_list.empty()) {
const int initial_origin = origin;
alignWithManufacturingGrid(origin);

if (!isOriginInsideCore(origin, target_direction)) {

@joaomai joaomai Oct 26, 2026

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.

Couldn't the same be achieved by changing the way alignWithManufacturingGrid rounds the origin value? In the sense that if rounding down fails, rounding up should pass and vice-versa.

// Grid rounding can move an initially contained macro past the core.
const odb::Rect& macro_bbox = inst_->getBBox()->getBox();
const odb::Rect& core = inst_->getBlock()->getCoreArea();
const int manufacturing_grid
= inst_->getDb()->getTech()->getManufacturingGrid();
Comment thread
joaomai marked this conversation as resolved.
Resolved
const int lower_bound
= initial_origin
+ (target_direction == odb::dbTechLayerDir::VERTICAL
? core.xMin() - macro_bbox.xMin()
: core.yMin() - macro_bbox.yMin());
const int upper_bound
= initial_origin
+ (target_direction == odb::dbTechLayerDir::VERTICAL
? core.xMax() - macro_bbox.xMax()
: core.yMax() - macro_bbox.yMax());
const int minimum_origin
= static_cast<int>(std::ceil(
lower_bound / static_cast<double>(manufacturing_grid)))
* manufacturing_grid;
const int maximum_origin
= static_cast<int>(std::floor(
upper_bound / static_cast<double>(manufacturing_grid)))
* manufacturing_grid;
Comment on lines +62 to +69

@gemini-code-assist gemini-code-assist Bot Oct 21, 2026

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.

medium

We can avoid floating-point conversions and potential precision/performance overhead by using pure integer arithmetic to compute minimum_origin and maximum_origin.

      const int minimum_origin
          = (lower_bound >= 0
                 ? (lower_bound + manufacturing_grid - 1) / manufacturing_grid
                 : lower_bound / manufacturing_grid)
            * manufacturing_grid;
      const int maximum_origin
          = (upper_bound >= 0
                 ? upper_bound / manufacturing_grid
                 : (upper_bound - manufacturing_grid + 1) / manufacturing_grid)
            * manufacturing_grid;


if (minimum_origin > maximum_origin) {
setOrigin(initial_origin, target_direction);
return;
}
origin = std::clamp(origin, minimum_origin, maximum_origin);
}

setOrigin(origin, target_direction);
return;
}
Expand All @@ -61,14 +95,33 @@ void Snapper::snap(const odb::dbTechLayerDir& target_direction)
starting_position_index -= 1;
}

snapPinToPosition(lowest_grid_pin,
lowest_grid_positions[starting_position_index],
target_direction);

attemptSnapToExtraPatterns(

@joaomai joaomai Oct 26, 2026

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.

This function can be renamed to something akin to "snapToGridPatterns" since the extra has no meaning now.

starting_position_index, layers_data_list, target_direction);
}

bool Snapper::isOriginInsideCore(
const int origin,
const odb::dbTechLayerDir& target_direction) const
{
const odb::Rect& macro_bbox = inst_->getBBox()->getBox();
const odb::Rect& core = inst_->getBlock()->getCoreArea();
if (core.dx() <= 0 || core.dy() <= 0) {
return true;
}

const int current_origin = target_direction == odb::dbTechLayerDir::VERTICAL
? inst_->getOrigin().x()
: inst_->getOrigin().y();
const int displacement = origin - current_origin;

if (target_direction == odb::dbTechLayerDir::VERTICAL) {
return macro_bbox.xMin() + displacement >= core.xMin()
&& macro_bbox.xMax() + displacement <= core.xMax();
}
return macro_bbox.yMin() + displacement >= core.yMin()
&& macro_bbox.yMax() + displacement <= core.yMax();
}

void Snapper::setOrigin(const int origin,
const odb::dbTechLayerDir& target_direction)
{
Expand Down Expand Up @@ -178,13 +231,19 @@ int Snapper::getPinOffset(odb::dbITerm* pin,
return pin_offset;
}

int Snapper::getSnappedOrigin(const int position, const int pin_offset)
{
int origin = position - pin_offset;
alignWithManufacturingGrid(origin);
return origin;
}

void Snapper::snapPinToPosition(odb::dbITerm* pin,
int position,
const int position,
const odb::dbTechLayerDir& direction)
{
int origin = position - getPinOffset(pin, direction);
alignWithManufacturingGrid(origin);
setOrigin(origin, direction);
setOrigin(getSnappedOrigin(position, getPinOffset(pin, direction)),
direction);
}

void Snapper::getTrackGridPattern(odb::dbTrackGrid* track_grid,
Expand Down Expand Up @@ -216,33 +275,45 @@ void Snapper::attemptSnapToExtraPatterns(

odb::dbITerm* snap_pin = layers_data_list[0].pins[0];
const std::vector<int>& positions = layers_data_list[0].available_positions;

int best_index = start_index;
int best_snapped_pins = 0;

std::vector<int> legal_positions;
legal_positions.reserve(total_attempts + 1);
const int pin_offset = getPinOffset(snap_pin, target_direction);
// Keep the existing nearest-first order, but cull before moving the macro.
for (int i = 0; i <= total_attempts; i++) {
int steps = (i % 2 == 1) ? (i + 1) / 2 : -(i / 2);

int current_index = start_index + steps;

if (current_index < 0
|| current_index >= layers_data_list[0].available_positions.size()) {
continue;
}
snapPinToPosition(snap_pin, positions[current_index], target_direction);
const int position = positions[current_index];
if (isOriginInsideCore(getSnappedOrigin(position, pin_offset),
target_direction)) {
legal_positions.push_back(position);
}
}
Comment on lines +278 to +294

@gemini-code-assist gemini-code-assist Bot Oct 21, 2026

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.

medium

The getPinOffset function is called 101 times inside the loop for the same snap_pin and target_direction, which is highly redundant. We can cache the pin offset before the loop to avoid these redundant calls and improve performance.

  std::vector<int> legal_positions;
  legal_positions.reserve(total_attempts + 1);
  const int pin_offset = getPinOffset(snap_pin, target_direction);
  // Keep the existing nearest-first order, but cull before moving the macro.
  for (int i = 0; i <= total_attempts; i++) {
    int steps = (i % 2 == 1) ? (i + 1) / 2 : -(i / 2);
    int current_index = start_index + steps;
    if (current_index < 0
        || current_index >= layers_data_list[0].available_positions.size()) {
      continue;
    }
    const int position = positions[current_index];
    int origin = position - pin_offset;
    alignWithManufacturingGrid(origin);
    if (isOriginInsideCore(origin, target_direction)) {
      legal_positions.push_back(position);
    }
  }

if (legal_positions.empty()) {
// Keeping the original location must not bypass mandatory pin-grid rules.
totalAlignedPins(layers_data_list, target_direction, true);
return;

@joaomai joaomai Oct 26, 2026

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.

Early returning here skips the warning down the function, which is undesirable.

}

int best_position = legal_positions.front();
int best_snapped_pins = -1;
for (const int position : legal_positions) {
snapPinToPosition(snap_pin, position, target_direction);
int snapped_pins = totalAlignedPins(layers_data_list, target_direction);

if (snapped_pins > best_snapped_pins) {
best_snapped_pins = snapped_pins;
best_index = current_index;
best_position = position;
if (best_snapped_pins == total_pins) {
break;
}
}
}

snapPinToPosition(snap_pin, positions[best_index], target_direction);
snapPinToPosition(snap_pin, best_position, target_direction);

if (best_snapped_pins != total_pins) {
totalAlignedPins(layers_data_list, target_direction, true);
Expand Down
3 changes: 3 additions & 0 deletions src/mpl/src/snapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class Snapper

void snap(const odb::dbTechLayerDir& target_direction);
void alignWithManufacturingGrid(int& origin);
bool isOriginInsideCore(int origin,
const odb::dbTechLayerDir& target_direction) const;
void setOrigin(int origin, const odb::dbTechLayerDir& target_direction);
int totalAlignedPins(const LayerDataList& layers_data_list,
const odb::dbTechLayerDir& direction,
Expand All @@ -61,6 +63,7 @@ class Snapper
int& step,
const odb::dbTechLayerDir& target_direction);
int getPinOffset(odb::dbITerm* pin, const odb::dbTechLayerDir& direction);
int getSnappedOrigin(int position, int pin_offset);
void snapPinToPosition(odb::dbITerm* pin,
int position,
const odb::dbTechLayerDir& direction);
Expand Down
8 changes: 4 additions & 4 deletions src/mpl/test/boundary_push1.defok
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ TRACKS Y 140 DO 282 STEP 3200 LAYER metal9 ;
TRACKS X 190 DO 282 STEP 3200 LAYER metal10 ;
TRACKS Y 140 DO 282 STEP 3200 LAYER metal10 ;
COMPONENTS 4 ;
- MACRO_1 HM_100x100_1x1 + FIXED ( 239900 242530 ) S ;
- MACRO_1 HM_100x100_1x1 + FIXED ( 239900 242250 ) S ;
- MACRO_2 HM_100x100_1x1 + FIXED ( 239900 330 ) S ;
- MACRO_3 HM_100x100_1x1 + FIXED ( 140 330 ) S ;
- MACRO_4 HM_100x100_1x1 + FIXED ( 140 242530 ) S ;
- MACRO_4 HM_100x100_1x1 + FIXED ( 140 242250 ) S ;
END COMPONENTS
PINS 4 ;
- io_1 + NET io_1 + DIRECTION INPUT + USE SIGNAL ;
Expand All @@ -196,9 +196,9 @@ PINS 4 ;
END PINS
BLOCKAGES 4 ;
- PLACEMENT + SOFT + COMPONENT MACRO_3 RECT ( 0 190 ) ( 200280 200470 ) ;
- PLACEMENT + SOFT + COMPONENT MACRO_4 RECT ( 0 242390 ) ( 200280 442670 ) ;
- PLACEMENT + SOFT + COMPONENT MACRO_4 RECT ( 0 242110 ) ( 200280 442390 ) ;
- PLACEMENT + SOFT + COMPONENT MACRO_2 RECT ( 239760 190 ) ( 440040 200470 ) ;
- PLACEMENT + SOFT + COMPONENT MACRO_1 RECT ( 239760 242390 ) ( 440040 442670 ) ;
- PLACEMENT + SOFT + COMPONENT MACRO_1 RECT ( 239760 242110 ) ( 440040 442390 ) ;
END BLOCKAGES
NETS 4 ;
- io_1 ( PIN io_1 ) + USE SIGNAL ;
Expand Down
171 changes: 171 additions & 0 deletions src/mpl/test/cpp/TestSnapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause
// Copyright (c) 2023-2025, The OpenROAD Authors

#include <array>
#include <exception>
#include <memory>
#include <string>
Expand Down Expand Up @@ -47,6 +48,176 @@ TEST_F(TestSnapper, ManufacturingGrid)
EXPECT_EQ(macro->getOrigin().y(), 1020);
}

// Keeps a macro inside a core that has no legal manufacturing-grid coordinate.
TEST_F(TestSnapper, ManufacturingGridPreservesCoreContainedOrigin)
{
db_->getTech()->setManufacturingGrid(10);
odb::dbBlock* block = db_->getChip()->getBlock();
block->setCoreArea(odb::Rect(5, 5, 105, 105));

odb::dbMaster* macro_master
= odb::dbMaster::create(db_->findLib("lib"), "macro_master");
macro_master->setWidth(100);
macro_master->setHeight(100);
macro_master->setType(odb::dbMasterType::BLOCK);
macro_master->setFrozen();

odb::dbInst* macro = odb::dbInst::create(block, macro_master, "macro");
macro->setOrigin(5, 5);
snapper_->setMacro(macro);
snapper_->snapMacro();

EXPECT_EQ(macro->getOrigin().x(), 5);
EXPECT_EQ(macro->getOrigin().y(), 5);
}

class TestSnapperCoreBoundary : public TestSnapper,
public testing::WithParamInterface<int>
{
protected:
odb::dbInst* makeMacro(bool pins, int pin_min = 4, int height = 100)
{
db_->getTech()->setManufacturingGrid(10);
auto* block = db_->getChip()->getBlock();
auto* master = odb::dbMaster::create(db_->findLib("lib"), "macro_master");
master->setWidth(100);
master->setHeight(height);
master->setType(odb::dbMasterType::BLOCK);
if (pins) {
for (const auto direction :
{odb::dbTechLayerDir::VERTICAL, odb::dbTechLayerDir::HORIZONTAL}) {
const char* name
= direction == odb::dbTechLayerDir::VERTICAL ? "V" : "H";
auto* layer = odb::dbTechLayer::create(
db_->getTech(), name, odb::dbTechLayerType::DEFAULT);
layer->setDirection(direction);
auto* grid = odb::dbTrackGrid::create(block, layer);
if (direction == odb::dbTechLayerDir::VERTICAL) {
grid->addGridPatternX(0, 22, 10);
} else {
grid->addGridPatternY(0, 22, 10);
}
auto* term = odb::dbMTerm::create(
master, name, odb::dbIoType::INPUT, odb::dbSigType::SIGNAL);
auto* pin = odb::dbMPin::create(term);
odb::dbBox::create(
pin, layer, pin_min, pin_min, pin_min + 2, pin_min + 2);
}
}
master->setFrozen();
auto* macro = odb::dbInst::create(block, master, "macro");
const std::array<odb::dbOrientType, 8> orientations{
odb::dbOrientType::R0,
odb::dbOrientType::R90,
odb::dbOrientType::R180,
odb::dbOrientType::R270,
odb::dbOrientType::MX,
odb::dbOrientType::MY,
odb::dbOrientType::MXR90,
odb::dbOrientType::MYR90};
macro->setOrient(orientations[GetParam()]);
snapper_->setMacro(macro);
return macro;
}
};

TEST_P(TestSnapperCoreBoundary, ManufacturingRoundingAtCoreEdges)
{
auto* macro = makeMacro(false);
auto* block = db_->getChip()->getBlock();
const odb::Rect core(5, 5, 205, 205);
block->setCoreArea(core);
for (const int x : {5, 105}) {
for (const int y : {5, 105}) {
SCOPED_TRACE(testing::Message() << "location=" << x << "," << y);
macro->setLocation(x, y);
ASSERT_TRUE(core.contains(macro->getBBox()->getBox()));
snapper_->snapMacro();
const auto& box = macro->getBBox()->getBox();
EXPECT_TRUE(core.contains(box));
EXPECT_EQ(box.xMin(), x == 5 ? 10 : 100);
EXPECT_EQ(box.yMin(), y == 5 ? 10 : 100);
}
}
}

TEST_P(TestSnapperCoreBoundary, NoManufacturingGridCoordinatePreservesOrigin)
{
auto* macro = makeMacro(false);
const odb::Rect core(5, 5, 105, 105);
macro->getBlock()->setCoreArea(core);
macro->setLocation(5, 5);
const auto origin = macro->getOrigin();
snapper_->snapMacro();
EXPECT_EQ(macro->getOrigin(), origin);
EXPECT_EQ(macro->getBBox()->getBox(), core);
}

TEST_P(TestSnapperCoreBoundary, TrackCandidateIncludesManufacturingRounding)
{
auto* macro = makeMacro(true);
const odb::Rect core(5, 5, 205, 205);
macro->getBlock()->setCoreArea(core);
macro->setLocation(105, 105);
ASSERT_TRUE(core.contains(macro->getBBox()->getBox()));
snapper_->snapMacro();
const auto& box = macro->getBBox()->getBox();
EXPECT_TRUE(core.contains(box));
EXPECT_EQ(box.xMin(), 100);
EXPECT_EQ(box.yMin(), 100);
}

TEST_P(TestSnapperCoreBoundary, NoLegalTrackCandidatePreservesOrigin)
{
auto* macro = makeMacro(true);
const odb::Rect core(5, 5, 105, 105);
macro->getBlock()->setCoreArea(core);
macro->setLocation(5, 5);
const auto origin = macro->getOrigin();
snapper_->snapMacro();
EXPECT_EQ(macro->getOrigin(), origin);
EXPECT_EQ(macro->getBBox()->getBox(), core);
}

TEST_P(TestSnapperCoreBoundary, NonsquareRotatedMacroStaysInsideCore)
{
auto* macro = makeMacro(true, 4, 60);
const odb::Rect core(5, 5, 205, 205);
macro->getBlock()->setCoreArea(core);
const int x = core.xMax() - macro->getBBox()->getBox().dx();
const int y = core.yMax() - macro->getBBox()->getBox().dy();
macro->setLocation(x, y);
ASSERT_TRUE(core.contains(macro->getBBox()->getBox()));
snapper_->snapMacro();
const auto& box = macro->getBBox()->getBox();
EXPECT_TRUE(core.contains(box));
EXPECT_EQ(box.xMin(), x - 5);
EXPECT_EQ(box.yMin(), y - 5);
}

TEST_P(TestSnapperCoreBoundary, NoLegalCandidateStillChecksRequiredGrid)
{
auto* macro = makeMacro(true, 0);
const odb::Rect core(5, 5, 105, 105);
macro->getBlock()->setCoreArea(core);
macro->setLocation(5, 5);
db_->getTech()->findLayer("H")->setRightWayOnGridOnly(true);
db_->getTech()->findLayer("V")->setRightWayOnGridOnly(true);
const auto origin = macro->getOrigin();
try {
snapper_->snapMacro();
FAIL() << "Expected an unsnappable RightWayOnGridOnly pin to fail";
} catch (const std::exception& error) {
EXPECT_STREQ(error.what(), "MPL-0005");
}
EXPECT_EQ(macro->getOrigin(), origin);
EXPECT_EQ(macro->getBBox()->getBox(), core);
}

INSTANTIATE_TEST_SUITE_P(AllOrientations,
TestSnapperCoreBoundary,
testing::Range(0, 8));

// Snaps a macro using one horizontal layer, aligning one pin
TEST_F(TestSnapper, SingleLayer)
{
Expand Down
Loading
Loading