Skip to content

dpl: avoid improve_placement crashes on no-site macros( improve_placement core dump) #9863

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

Closed
duckkkkkkkkking wants to merge 1 commit into from
Closed

dpl: avoid improve_placement crashes on no-site macros( improve_placement core dump) #9863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/dpl/src/infrastructure/Grid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ GridY Grid::gridHeight(odb::dbMaster* master) const
return GridY{max(1, divCeil(bbox.dy(), row_height.v))};
}
auto site = master->getSite();
if (!site->hasRowPattern()) {
if (site == nullptr || !site->hasRowPattern()) {
return GridY{1};
}

Expand All @@ -548,7 +548,7 @@ GridY Grid::gridHeight(const Node* cell) const
return GridY{1};
}
auto site = cell->getDbInst()->getMaster()->getSite();
if (!site->hasRowPattern()) {
if (site == nullptr || !site->hasRowPattern()) {
return GridY{1};
}

Expand Down
42 changes: 30 additions & 12 deletions src/dpl/src/optimization/detailed_manager.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ static bool checkMasterSymmetry(Architecture* arch, const Node* nd, int rowId)
return false;
}

namespace {
bool usesRowSites(const Node* node)
{
return node->getSite() != nullptr;
}

void syncOrientToRowSite(Grid* grid, Node* node, GridX grid_x, GridY grid_y)
{
auto* site = node->getSite();
if (site == nullptr) {
return;
}
if (const auto orientation = grid->getSiteOrientation(grid_x, grid_y, site)) {
node->adjustCurrOrient(*orientation);
}
}
} // namespace
DetailedMgr::DetailedMgr(Architecture* arch,
Network* network,
Grid* grid,
Expand Down Expand Up @@ -1059,7 +1076,7 @@ void DetailedMgr::collectSingleHeightCells()
for (int i = 0; i < network_->getNumNodes(); i++) {
Node* nd = network_->getNode(i);

if (nd->isTerminal() || nd->isFixed()) {
if (nd->isTerminal() || nd->isFixed() || !usesRowSites(nd)) {
continue;
}
if (arch_->isMultiHeightCell(nd)) {
Expand Down Expand Up @@ -1097,7 +1114,8 @@ void DetailedMgr::collectMultiHeightCells()
for (int i = 0; i < network_->getNumNodes(); i++) {
Node* nd = network_->getNode(i);

if (nd->isTerminal() || nd->isFixed() || arch_->isSingleHeightCell(nd)) {
if (nd->isTerminal() || nd->isFixed() || !usesRowSites(nd)
|| arch_->isSingleHeightCell(nd)) {
continue;
}

Expand Down Expand Up @@ -1134,7 +1152,10 @@ void DetailedMgr::collectFixedCells()
for (int i = 0; i < network_->getNumNodes(); i++) {
Node* nd = network_->getNode(i);

if (nd->isFixed()) {
// Instances without a row site cannot participate in row-based detailed
// placement. Treat them as fixed obstacles so legalization still honors
// their occupied area.
if (nd->isFixed() || (!nd->isTerminal() && !usesRowSites(nd))) {
fixedCells_.push_back(nd);
}
}
Expand Down Expand Up @@ -1468,14 +1489,15 @@ int DetailedMgr::checkRegionAssignment()
int DetailedMgr::checkSiteAlignment()
{
// Ensure that the left edge of each cell is aligned with a site. We only
// consider cells that are within segments.
// consider cells that are within segments. Instances without row sites are
// handled as fixed obstacles and are excluded from this row-based check.
int err_n = 0;

const DbuY singleRowHeight = getSingleRowHeight();
for (int i = 0; i < network_->getNumNodes(); i++) {
const Node* nd = network_->getNode(i);

if (nd->isTerminal() || nd->isFixed()) {
if (nd->isTerminal() || nd->isFixed() || !usesRowSites(nd)) {
continue;
}

Expand Down Expand Up @@ -1525,13 +1547,13 @@ int DetailedMgr::checkSiteAlignment()
////////////////////////////////////////////////////////////////////////////////
int DetailedMgr::checkRowAlignment()
{
// Ensure that the bottom of each cell is aligned with a row.
// Ensure that the bottom of each row-based cell is aligned with a row.
int err_n = 0;

for (int i = 0; i < network_->getNumNodes(); i++) {
const Node* nd = network_->getNode(i);

if (nd->isTerminal() || nd->isFixed()) {
if (nd->isTerminal() || nd->isFixed() || !usesRowSites(nd)) {
continue;
}

Expand Down Expand Up @@ -3047,11 +3069,7 @@ void DetailedMgr::paintInGrid(Node* node)
{
const auto grid_x = grid_->gridX(node);
const auto grid_y = grid_->gridSnapDownY(node);
odb::dbSite* site = node->getDbInst()->getMaster()->getSite();

const auto orientation
= grid_->getSiteOrientation(grid_x, grid_y, site).value();
grid_->paintPixel(node, grid_x, grid_y);
node->adjustCurrOrient(orientation);
syncOrientToRowSite(grid_, node, grid_x, grid_y);
}
} // namespace dpl
55 changes: 32 additions & 23 deletions src/dpl/src/optimization/detailed_orient.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ using utl::DPL;

namespace dpl {

namespace {
bool usesRowSites(const Node* node)
{
return node->getSite() != nullptr;
}
} // namespace
Comment on lines +29 to +34
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 20, 2026

Choose a reason for hiding this comment

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

medium

This usesRowSites helper function is also defined in src/dpl/src/optimization/detailed_manager.cxx. To avoid code duplication, consider moving this logic into a method on the Node class itself, for example bool Node::hasSite() const in Objects.h. This would improve code reuse and maintainability.

References
  1. Encapsulate logic to avoid code duplication.


DetailedOrient::DetailedOrient(Architecture* arch, Network* network)
: arch_(arch),
network_(network),
Expand Down Expand Up @@ -119,35 +126,37 @@ int DetailedOrient::orientCells(int& changed)
int errors = 0;
for (int i = 0; i < network_->getNumNodes(); i++) {
Node* ndi = network_->getNode(i);
if (!(ndi->isTerminal() || ndi->isFixed())) {
// Figure out the lowest row for the cell. Not that single
// height cells are only in one row.
int bottom = arch_->getNumRows();
for (int s = 0; s < mgrPtr_->getNumReverseCellToSegs(ndi->getId()); s++) {
DetailedSeg* segPtr = mgrPtr_->getReverseCellToSegs(ndi->getId())[s];
bottom = std::min(bottom, segPtr->getRowId());
}
if (bottom < arch_->getNumRows()) {
unsigned origOrient = ndi->getOrient();
if (arch_->isSingleHeightCell(ndi)) {
if (!orientSingleHeightCellForRow(ndi, bottom)) {
++errors;
}
} else if (arch_->isMultiHeightCell(ndi)) {
if (!orientMultiHeightCellForRow(ndi, bottom)) {
++errors;
}
} else {
// ? Whoops.
if (ndi->isTerminal() || ndi->isFixed() || !usesRowSites(ndi)) {
continue;
}

// Figure out the lowest row for the cell. Not that single
// height cells are only in one row.
int bottom = arch_->getNumRows();
for (int s = 0; s < mgrPtr_->getNumReverseCellToSegs(ndi->getId()); s++) {
DetailedSeg* segPtr = mgrPtr_->getReverseCellToSegs(ndi->getId())[s];
bottom = std::min(bottom, segPtr->getRowId());
}
if (bottom < arch_->getNumRows()) {
unsigned origOrient = ndi->getOrient();
if (arch_->isSingleHeightCell(ndi)) {
if (!orientSingleHeightCellForRow(ndi, bottom)) {
++errors;
}
if (origOrient != ndi->getOrient()) {
++changed;
} else if (arch_->isMultiHeightCell(ndi)) {
if (!orientMultiHeightCellForRow(ndi, bottom)) {
++errors;
}
} else {
// Cell not in a row? Whoops.
// ? Whoops.
++errors;
}
if (origOrient != ndi->getOrient()) {
++changed;
}
} else {
// Cell not in a row? Whoops.
++errors;
}
}
return errors;
Expand Down
11 changes: 7 additions & 4 deletions src/dpl/src/util/journal.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ void paintInGrid(Grid* grid, Node* node)
{
const auto grid_x = grid->gridX(DbuX(node->getLeft()));
const auto grid_y = grid->gridRoundY(DbuY(node->getBottom()));
odb::dbSite* site = node->getDbInst()->getMaster()->getSite();
const auto orientation
= grid->getSiteOrientation(grid_x, grid_y, site).value();
grid->paintPixel(node, grid_x, grid_y);
node->adjustCurrOrient(orientation);
auto* site = node->getSite();
if (site == nullptr) {
return;
}
if (const auto orientation = grid->getSiteOrientation(grid_x, grid_y, site)) {
node->adjustCurrOrient(*orientation);
}
Comment on lines +26 to +32
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 20, 2026

Choose a reason for hiding this comment

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

medium

This logic for safely updating a node's orientation is also implemented in src/dpl/src/optimization/detailed_manager.cxx as the syncOrientToRowSite helper function. To avoid this duplication, you could move syncOrientToRowSite to a shared utility header (e.g., in src/dpl/src/util/) and call it from both locations.

References
  1. Encapsulate logic to avoid code duplication.

}

}; // namespace
Expand Down
4 changes: 4 additions & 0 deletions src/dpl/test/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ COMPULSORY_TESTS = [
"gcd_no_one_site_gaps-opt",
"ibex-opt",
"multi_height1-opt",
"no_site_block-opt",
"edge_spacing-opt",
"regions1-opt",
"regions2-opt",
Expand Down Expand Up @@ -233,6 +234,9 @@ filegroup(
"multi_height1-opt": [
"Nangate45/fake_macros.lef",
],
"no_site_block-opt": [
"no_site_block.lef",
],
"multi_height_one_site_gap_disallow": [
"Nangate45/fake_macros.lef",
"Nangate45_data/Nangate45.lef",
Expand Down
2 changes: 1 addition & 1 deletion src/dpl/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ or_integration_tests(
gcd_no_one_site_sanity
ibex-opt
multi_height1-opt
no_site_block-opt
edge_spacing-opt
regions1-opt
regions2-opt
Expand All @@ -100,4 +101,3 @@ gtest_discover_tests(dpl_test
)

add_dependencies(build_and_test dpl_test)

79 changes: 79 additions & 0 deletions src/dpl/test/no_site_block-opt.def
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
VERSION 5.8 ;
DIVIDERCHAR "/" ;
BUSBITCHARS "[]" ;
DESIGN no_site_block ;
UNITS DISTANCE MICRONS 2000 ;
DIEAREA ( 0 0 ) ( 20000 20000 ) ;
ROW ROW_0 FreePDK45_38x28_10R_NP_162NW_34O 3800 2800 FS DO 32 BY 1 STEP 380 0 ;
ROW ROW_1 FreePDK45_38x28_10R_NP_162NW_34O 3800 5600 N DO 32 BY 1 STEP 380 0 ;
ROW ROW_2 FreePDK45_38x28_10R_NP_162NW_34O 3800 8400 FS DO 32 BY 1 STEP 380 0 ;
ROW ROW_3 FreePDK45_38x28_10R_NP_162NW_34O 3800 11200 N DO 32 BY 1 STEP 380 0 ;
TRACKS X 190 DO 52 STEP 380 LAYER metal1 ;
TRACKS Y 140 DO 70 STEP 280 LAYER metal1 ;
TRACKS X 190 DO 52 STEP 380 LAYER metal2 ;
TRACKS Y 140 DO 70 STEP 280 LAYER metal2 ;
TRACKS X 190 DO 52 STEP 380 LAYER metal3 ;
TRACKS Y 140 DO 70 STEP 280 LAYER metal3 ;
TRACKS X 190 DO 35 STEP 560 LAYER metal4 ;
TRACKS Y 140 DO 35 STEP 560 LAYER metal4 ;
TRACKS X 190 DO 35 STEP 560 LAYER metal5 ;
TRACKS Y 140 DO 35 STEP 560 LAYER metal5 ;
TRACKS X 190 DO 35 STEP 560 LAYER metal6 ;
TRACKS Y 140 DO 35 STEP 560 LAYER metal6 ;
TRACKS X 190 DO 12 STEP 1600 LAYER metal7 ;
TRACKS Y 140 DO 12 STEP 1600 LAYER metal7 ;
TRACKS X 190 DO 12 STEP 1600 LAYER metal8 ;
TRACKS Y 140 DO 12 STEP 1600 LAYER metal8 ;
TRACKS X 190 DO 6 STEP 3200 LAYER metal9 ;
TRACKS Y 140 DO 6 STEP 3200 LAYER metal9 ;
TRACKS X 190 DO 6 STEP 3200 LAYER metal10 ;
TRACKS Y 140 DO 6 STEP 3200 LAYER metal10 ;
VIAS 3 ;
- via4_960x2800 + VIARULE Via4Array-0 + CUTSIZE 280 280 + LAYERS metal4 metal5 via4 + CUTSPACING 320 320 + ENCLOSURE 40 60 40 60 + ROWCOL 5 2 ;
- via5_960x2800 + VIARULE Via5Array-0 + CUTSIZE 280 280 + LAYERS metal5 metal6 via5 + CUTSPACING 320 320 + ENCLOSURE 40 60 40 60 + ROWCOL 5 2 ;
- via6_960x2800 + VIARULE Via6Array-0 + CUTSIZE 280 280 + LAYERS metal6 metal7 via6 + CUTSPACING 320 320 + ENCLOSURE 340 360 340 360 + ROWCOL 4 1 ;
END VIAS
COMPONENTS 1 ;
- macro1 NO_SITE_BLOCK + PLACED ( 7600 2800 ) N ;
END COMPONENTS
PINS 1 ;
- input + NET input + DIRECTION INPUT + USE SIGNAL + FIXED ( 0 -140 ) N + LAYER metal6 ( -140 -140 ) ( 140 140 ) ;
END PINS
SPECIALNETS 2 ;
- VDD ( * VDD ) + USE POWER
+ ROUTED metal6 0 + SHAPE STRIPE ( 136140 106400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 136140 106400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 136140 106400 ) via4_960x2800
NEW metal6 0 + SHAPE STRIPE ( 24140 106400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 24140 106400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 24140 106400 ) via4_960x2800
NEW metal6 0 + SHAPE STRIPE ( 136140 26400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 136140 26400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 136140 26400 ) via4_960x2800
NEW metal6 0 + SHAPE STRIPE ( 24140 26400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 24140 26400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 24140 26400 ) via4_960x2800
NEW metal7 2800 + SHAPE STRIPE ( 20140 106400 ) ( 180120 106400 )
NEW metal7 2800 + SHAPE STRIPE ( 20140 26400 ) ( 180120 26400 )
NEW metal4 960 + SHAPE STRIPE ( 136140 22400 ) ( 136140 179200 )
NEW metal4 960 + SHAPE STRIPE ( 24140 22400 ) ( 24140 179200 )
NEW metal1 340 + SHAPE FOLLOWPIN ( 3800 14000 ) ( 15960 14000 )
NEW metal1 340 + SHAPE FOLLOWPIN ( 3800 8400 ) ( 15960 8400 )
NEW metal1 340 + SHAPE FOLLOWPIN ( 3800 2800 ) ( 15960 2800 ) ;
- VSS ( * VSS ) + USE GROUND
+ ROUTED metal6 0 + SHAPE STRIPE ( 80140 146400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 80140 146400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 80140 146400 ) via4_960x2800
NEW metal6 0 + SHAPE STRIPE ( 80140 66400 ) via6_960x2800
NEW metal5 0 + SHAPE STRIPE ( 80140 66400 ) via5_960x2800
NEW metal4 0 + SHAPE STRIPE ( 80140 66400 ) via4_960x2800
NEW metal7 2800 + SHAPE STRIPE ( 20140 146400 ) ( 180120 146400 )
NEW metal7 2800 + SHAPE STRIPE ( 20140 66400 ) ( 180120 66400 )
NEW metal4 960 + SHAPE STRIPE ( 80140 22400 ) ( 80140 179200 )
NEW metal1 340 + SHAPE FOLLOWPIN ( 3800 11200 ) ( 15960 11200 )
NEW metal1 340 + SHAPE FOLLOWPIN ( 3800 5600 ) ( 15960 5600 ) ;
END SPECIALNETS
NETS 1 ;
- input ( PIN input ) ( macro1 A ) + USE SIGNAL ;
END NETS
END DESIGN
Loading