The-OpenROAD-Project / The-OpenROAD-Project/OpenROAD
dpl: checkOneSiteGap is implemented twice and the two disagree at the core edge
@gudeh is already working on this.
Since Sep 3, 2026.
- Dominant language
- Verilog
- Stars
- 3.1k
- Forks
- 1k
- Avg merge
- 2d 23h
- Merged PRs (30d)
- 136
Description
While evaluating dpl's placement checks we noticed that the one-site-gap rule is implemented
twice, and that the two copies differ in one lambda.
Opendp::checkPixels (src/dpl/src/Place.cpp):
auto isAbutted = [this](const GridX x, const GridY y) {
const Pixel* pixel = grid_->gridPixel(x, y);
return (pixel == nullptr || pixel->cell);
};
auto cellAtSite = [this](const GridX x, const GridY y) {
const Pixel* pixel = grid_->gridPixel(x, y);
return (pixel != nullptr && pixel->cell);
};
PlacementDRC::checkOneSiteGap (src/dpl/src/PlacementDRC.cpp):
auto isAbutted = [this](const GridX x, const GridY y) {
const Pixel* pixel = grid_->gridPixel(x, y);
return (pixel == nullptr || pixel->cell);
};
auto cellAtSite = [this](const GridX x, const GridY y) {
const Pixel* pixel = grid_->gridPixel(x, y);
return (pixel == nullptr || pixel->cell);
};
In the second, cellAtSite is byte-identical to isAbutted immediately above it.
The two spellings differ only for a nullptr pixel — off the grid, i.e. past the core edge. The
loop is
if (!isAbutted(x_begin, y) && cellAtSite(x_begin - 1, y)) { /* violation */ }
if (!isAbutted(x_finish, y) && cellAtSite(x_finish + 1, y)) { /* violation */ }
so with PlacementDRC's version a cell whose neighbouring site is empty and whose next-but-one
site is off the grid reports a one-site-gap violation, because off-grid is being read as "a cell
is there". With Place.cpp's version the same placement is clean. That is a cell placed one empty
site from the edge of the core.
We have not produced a failing design for it — the check is only active when
disallow_one_site_gap_ is set, and the geometry has to put the cell exactly one site from the
core boundary — so we are reporting the inconsistency rather than a reproducer.
Our reading is that Place.cpp's is the intended one: "is there a cell at this site" answering
true for a site that does not exist looks like the copy-paste rather than the rule. But the two
files disagree either way, and checkDRC and checkPlacement can therefore give different verdicts
on the same placement, which seems worth resolving in whichever direction you prefer.
Happy to open a PR aligning PlacementDRC's cellAtSite with Place.cpp's, with a test that
places a cell one site from the core edge and asserts both paths agree.
Observed on 945a9f48dc6e5cc91d865daa92c45a1094cb682c.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.