Skip to content

mpl: change objects that ared considered during notch calculation #9449

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
openroad-ci wants to merge 15 commits into
base: master
Choose a base branch
from
Open

mpl: change objects that ared considered during notch calculation #9449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f75c77f
mpl: include only macro and mixed clusters for notch calculation
joaomai Feb 9, 2026
b143b41
mpl: improve calNotchPenalty performance
joaomai Feb 9, 2026
e50af31
mpl: draw notches last
joaomai Feb 10, 2026
23b46fe
mpl: visited flag and avoid points collapse for notch calc
joaomai Feb 10, 2026
d599b29
mpl: better naming and comments for notch code
joaomai Feb 10, 2026
e0965f4
mpl: add const and static to funcs
joaomai Feb 10, 2026
c0bc1a3
mpl: update tests
joaomai Feb 10, 2026
78ccfcf
mpl: split isSegmentEmpty in two functions
joaomai Feb 10, 2026
e0abf02
Merge branch 'master' into mpl-notch-fix
joaomai Feb 12, 2026
2d91bc4
mpl: remove unused comment
joaomai Feb 12, 2026
5a105a1
fix typo
joaomai Feb 12, 2026
837aa74
Merge pull request #3128 from The-OpenROAD-Project-private/update-sta
maliberty Feb 14, 2026
fa5d170
Merge branch 'master' into mpl-notch-fix
joaomai Feb 15, 2026
2dfef67
Merge branch 'master' into mpl-notch-fix
joaomai Feb 16, 2026
82b70dc
Merge branch 'master' into mpl-notch-fix
joaomai Feb 17, 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
127 changes: 74 additions & 53 deletions src/mpl/src/SACoreSoftMacro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ void SACoreSoftMacro::fillCoordsLists(std::vector<int>& x_coords,
std::vector<int> y_point;

for (auto& macro : macros_) {
if (macro.isStdCellCluster()) {
if (!macro.isMacroCluster() && !macro.isMixedCluster()) {
continue;
}
x_point.push_back(macro.getX());
Expand All @@ -647,86 +647,98 @@ void SACoreSoftMacro::fillCoordsLists(std::vector<int>& x_coords,
std::ranges::sort(x_point);
std::ranges::sort(y_point);

// getSegmentIndex requires the bigger value
// from points within the same epsilon to work properly
int epsilon = outline_.dx() / 100;
for (int i = 0; i < x_point.size(); i++) {
if (i + 1 < x_point.size()
&& std::abs(x_point[i + 1] - x_point[i]) <= epsilon) {
continue;
x_coords.push_back(x_point.back());
for (int i = x_point.size() - 2; i >= 0; i--) {
if (x_coords.back() - x_point[i] > epsilon) {
x_coords.push_back(x_point[i]);
}
x_coords.push_back(x_point[i]);
}
std::ranges::reverse(x_coords);

epsilon = outline_.dy() / 100;
for (int i = 0; i < y_point.size(); i++) {
if (i + 1 < y_point.size()
&& std::abs(y_point[i + 1] - y_point[i]) <= epsilon) {
continue;
y_coords.push_back(y_point.back());

for (int i = y_point.size() - 2; i >= 0; i--) {
if (y_coords.back() - y_point[i] > epsilon) {
y_coords.push_back(y_point[i]);
}
y_coords.push_back(y_point[i]);
}
std::ranges::reverse(y_coords);
}

SACoreSoftMacro::Neighbors SACoreSoftMacro::findNeighbors(
std::vector<std::vector<bool>>& grid,
int start_row,
int start_col,
int end_row,
int end_col)
SACoreSoftMacro::NotchVicinity SACoreSoftMacro::checkNotchVicinity(
const std::vector<std::vector<bool>>& grid,
const int start_row,
const int start_col,
const int end_row,
const int end_col)
{
int num_y = grid.size();
int num_x = grid.front().size();

Neighbors neighbors;
NotchVicinity vicinity;
if (start_row > 0) {
for (int i = start_col; i <= end_col; i++) {
if (!grid[start_row - 1][i]) {
neighbors.bottom = false;
vicinity.bottom = false;
break;
}
}
}
if (end_row < num_y - 1) {
for (int i = start_col; i <= end_col; i++) {
if (!grid[end_row + 1][i]) {
neighbors.top = false;
vicinity.top = false;
break;
}
}
}
if (start_col > 0) {
for (int i = start_row; i <= end_row; i++) {
if (!grid[i][start_col - 1]) {
neighbors.left = false;
vicinity.left = false;
break;
}
}
}
if (end_col < num_x - 1) {
for (int i = start_row; i <= end_row; i++) {
if (!grid[i][end_col + 1]) {
neighbors.right = false;
vicinity.right = false;
break;
}
}
}

return neighbors;
return vicinity;
}

bool SACoreSoftMacro::isSegmentEmpty(std::vector<std::vector<bool>>& grid,
int start_row,
int start_col,
int end_row,
int end_col)
bool SACoreSoftMacro::isRowEmpty(const std::vector<std::vector<bool>>& grid,
const int row,
const int start_col,
const int end_col)
{
for (int i = start_row; i <= end_row; i++) {
for (int j = start_col; j <= end_col; j++) {
if (grid[i][j]) {
return false;
}
for (int col = start_col; col <= end_col; col++) {
if (grid[row][col]) {
return false;
}
}
return true;
}

bool SACoreSoftMacro::isColEmpty(const std::vector<std::vector<bool>>& grid,
const int col,
const int start_row,
const int end_row)
{
for (int row = start_row; row <= end_row; row++) {
if (grid[row][col]) {
return false;
}
}
return true;
}

Expand Down Expand Up @@ -779,7 +791,7 @@ void SACoreSoftMacro::calNotchPenalty()

std::vector<std::vector<bool>> grid(num_y, std::vector<bool>(num_x, false));
for (auto& macro : macros_) {
if (macro.isStdCellCluster()) {
if (!macro.isMacroCluster() && !macro.isMixedCluster()) {
Comment on lines -782 to +794
Copy link
Member

@maliberty maliberty Feb 11, 2026

Choose a reason for hiding this comment

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

What is the semantic difference?

Copy link
Contributor

@joaomai joaomai Feb 11, 2026

Choose a reason for hiding this comment

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

Using isMacroCluster keeps PADs, IOs, etc., which is undesired. I'll be opening an issue to better categorize the cluster "types" beyond standard cells, mixed and macro.

continue;
}
int x_start = getSegmentIndex(macro.getX(), x_coords);
Expand All @@ -793,30 +805,33 @@ void SACoreSoftMacro::calNotchPenalty()
}
}

std::vector<std::vector<bool>> visited(num_y,
std::vector<bool>(num_x, false));

for (int start_row = 0; start_row < num_y; start_row++) {
for (int start_col = 0; start_col < num_x; start_col++) {
if (grid[start_row][start_col]) {
if (grid[start_row][start_col] || visited[start_row][start_col]) {
continue;
}

int end_row = start_row;
int end_col = start_col;

Neighbors current_neighbors
= findNeighbors(grid, start_row, start_col, end_row, end_col);
NotchVicinity current_vicinity
= checkNotchVicinity(grid, start_row, start_col, end_row, end_col);
bool expand_rows = true;
bool expand_cols = true;

while (expand_rows || expand_cols) {
if (expand_rows) {
end_row += 1;
if (end_row < num_y
&& isSegmentEmpty(grid, start_row, start_col, end_row, end_col)) {
Neighbors expanded_neighbors
= findNeighbors(grid, start_row, start_col, end_row, end_col);
if (expanded_neighbors.total() > current_neighbors.total()
|| expanded_neighbors == current_neighbors) {
current_neighbors = expanded_neighbors;
&& isRowEmpty(grid, end_row, start_col, end_col)) {
NotchVicinity expanded_vicinity = checkNotchVicinity(
grid, start_row, start_col, end_row, end_col);
if (expanded_vicinity.total() > current_vicinity.total()
|| expanded_vicinity == current_vicinity) {
current_vicinity = expanded_vicinity;
} else {
expand_rows = false;
end_row -= 1;
Expand All @@ -830,12 +845,12 @@ void SACoreSoftMacro::calNotchPenalty()
if (expand_cols) {
end_col += 1;
if (end_col < num_x
&& isSegmentEmpty(grid, start_row, start_col, end_row, end_col)) {
Neighbors expanded_neighbors
= findNeighbors(grid, start_row, start_col, end_row, end_col);
if (expanded_neighbors.total() > current_neighbors.total()
|| expanded_neighbors == current_neighbors) {
current_neighbors = expanded_neighbors;
&& isColEmpty(grid, end_col, start_row, end_row)) {
NotchVicinity expanded_vicinity = checkNotchVicinity(
grid, start_row, start_col, end_row, end_col);
if (expanded_vicinity.total() > current_vicinity.total()
|| expanded_vicinity == current_vicinity) {
current_vicinity = expanded_vicinity;
} else {
expand_cols = false;
end_col -= 1;
Expand All @@ -847,17 +862,23 @@ void SACoreSoftMacro::calNotchPenalty()
}
}

for (int i = start_row; i < end_row + 1; i++) {
for (int j = start_col; j < end_col + 1; j++) {
visited[i][j] = true;
}
}

width = x_coords[end_col + 1] - x_coords[start_col];
height = y_coords[end_row + 1] - y_coords[start_row];

bool is_notch = false;
if (current_neighbors.total() == 4) {
is_notch = true;
} else if (current_neighbors.top && current_neighbors.bottom) {
if (current_vicinity.top && current_vicinity.bottom) {
if (height < notch_h_th_) {
is_notch = true;
}
} else if (current_neighbors.left && current_neighbors.right) {
}

if (current_vicinity.left && current_vicinity.right) {
if (width < notch_v_th_) {
is_notch = true;
}
Comment on lines 874 to 884
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 10, 2026

Choose a reason for hiding this comment

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

high

The logic for identifying a notch has been changed significantly. The original code considered any fully enclosed empty space as a notch (current_neighbors.total() == 4). This condition has been removed.

The new logic is:

bool is_notch = false;
if (current_vicinity.top && current_vicinity.bottom) {
  if (height < notch_h_th_) {
    is_notch = true;
  }
}

if (current_vicinity.left && current_vicinity.right) {
  if (width < notch_v_th_) {
    is_notch = true;
  }
}

For a fully enclosed space, this will only flag it as a notch if it is thin in at least one dimension (height < notch_h_th_ or width < notch_v_th_). A large, fully enclosed empty area will no longer be considered a notch, which seems incorrect as it represents wasted space that should be penalized.

This change is not mentioned in the PR description and appears to be a bug. If this change is intentional, it should be documented. Otherwise, the original logic for fully enclosed spaces should be restored.

      bool is_notch = false;
      if (current_vicinity.total() == 4) {
        is_notch = true;
      } else if (current_vicinity.top && current_vicinity.bottom) {
        if (height < notch_h_th_) {
          is_notch = true;
        }
      } else if (current_vicinity.left && current_vicinity.right) {
        if (width < notch_v_th_) {
          is_notch = true;
        }
      }

Expand Down
30 changes: 17 additions & 13 deletions src/mpl/src/SACoreSoftMacro.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ class SACoreSoftMacro : public SimulatedAnnealingCore<SoftMacro>
void forceCentralization() { force_centralization_ = true; }

private:
// Used to calculate notches
struct Neighbors
// Used to check the vicinity of candidate notches
struct NotchVicinity
{
bool top = true;
bool bottom = true;
Expand All @@ -84,7 +84,7 @@ class SACoreSoftMacro : public SimulatedAnnealingCore<SoftMacro>

int total() { return top + bottom + left + right; }

bool operator==(const Neighbors&) const = default;
bool operator==(const NotchVicinity&) const = default;
};

float calNormCost() const override;
Expand All @@ -100,16 +100,20 @@ class SACoreSoftMacro : public SimulatedAnnealingCore<SoftMacro>

void calBoundaryPenalty();
void fillCoordsLists(std::vector<int>& x_coords, std::vector<int>& y_coords);
Neighbors findNeighbors(std::vector<std::vector<bool>>& grid,
int start_row,
int start_col,
int end_row,
int end_col);
bool isSegmentEmpty(std::vector<std::vector<bool>>& grid,
int start_row,
int tart_col,
int end_row,
int end_col);
static NotchVicinity checkNotchVicinity(
const std::vector<std::vector<bool>>& grid,
int start_row,
int start_col,
int end_row,
int end_col);
static bool isRowEmpty(const std::vector<std::vector<bool>>& grid,
int row,
int start_col,
int end_col);
static bool isColEmpty(const std::vector<std::vector<bool>>& grid,
int col,
int start_row,
int end_row);
float calSingleNotchPenalty(int width, int height);
void calNotchPenalty();
void calMacroBlockagePenalty();
Expand Down
5 changes: 4 additions & 1 deletion src/mpl/src/graphics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,6 @@ void Graphics::drawObjects(gui::Painter& painter)
// Draw blockages and notches only during SA for SoftMacros
if (!soft_macros_.empty()) {
drawAllBlockages(painter);
drawNotches(painter);
}

painter.setPen(gui::Painter::kWhite, true);
Expand Down Expand Up @@ -547,6 +546,10 @@ void Graphics::drawObjects(gui::Painter& painter)
drawGuides(painter);
drawFences(painter);
}

if (!soft_macros_.empty()) {
drawNotches(painter);
}
}

template <typename T>
Expand Down
Loading