gpl: vertically merge unusable site dummy instances

[oharboe]

OpenROAD creates a dummy Instance for every single row a blocked region intersects. On macro-heavy designs (e.g. large RISC-V cores / MegaBoom-style configurations with large macro arrays) or complex floorplan blockages, this generates tens of thousands of single-row dummy instances if the chicken bones fall awkwardly.

During Nesterov global placement density calculations (updateBinsGCellDensityArea), iterating over these fragmented dummies causes an $O(N \times \text{Bins})$ CPU time explosion that can stall global placement for hours.

This patch vertically merges contiguous blocked site segments across rows into a single taller dummy instance and marks the merged site grid locations as SiteInfo::FixedInst so they are not double-processed. This reduces dummy instance count by orders of magnitude (e.g. from hundreds of thousands down to hundreds of instances) and drastically speeds up initial Nesterov iterations on macro-heavy designs.

Signed-off-by: Øyvind Harboe oyvind.harboe@zylin.com

[gemini-code-assist]

Code Review

This pull request updates PlacerBase::initInstsForUnusableSites in src/gpl/src/placerBase.cpp to vertically merge adjacent blocked sites into a single larger dummy instance. The reviewer suggested a cleaner implementation that defers marking the grid until after the vertical merge search is complete, which simplifies the logic and avoids duplicating the grid marking loops.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces vertical merging logic for unusable sites in PlacerBase::initInstsForUnusableSites() to combine adjacent blocked sites vertically into larger dummy instances. The reviewer suggests optimizing and simplifying this merging logic by hoisting 1D index calculations out of the inner loops to avoid redundant multiplications and refactoring the loop control for better readability.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates PlacerBase::initInstsForUnusableSites to group blocked sites vertically across multiple rows, creating larger dummy gcells. The reviewer suggested simplifying the nested loops and manual flag management by utilizing standard library algorithms like std::all_of and std::fill to improve code readability and maintainability.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request optimizes the handling of unusable sites in PlacerBase::initInstsForUnusableSites by merging vertically adjacent blocked sites into larger single dummy instances, rather than creating individual instances per row. The reviewer suggested using the explicit enum scope SiteInfo::Blocked instead of Blocked for consistency with SiteInfo::FixedInst and to improve robustness.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates PlacerBase::initInstsForUnusableSites in src/gpl/src/placerBase.cpp to merge vertically contiguous blocked sites into a single larger dummy instance instead of creating individual instances per row. It scans vertically to find matching blocked spans, fills the grid with SiteInfo::FixedInst, and instantiates the merged cell. There are no review comments, so we have no feedback to provide.

[oharboe]

@gudeh @maliberty Please retrigger:

worker-dind-foss-us-central1-65c34a11-sth1w-mqdjn has been removed or offline
  for 5 min 0 sec; assuming it is not coming back, and terminating shell step
    ...
    Timeout waiting for agent to come back
    Finished: ABORTED

[LucasYuki]

During Nesterov global placement density calculations (updateBinsGCellDensityArea), iterating over these fragmented dummies causes an O ( N × Bins ) CPU time explosion that can stall global placement for hours.

We only check the movable cells during updateBinsGCellDensityArea, and the overlapping calculation is done in O(1) by the getDensityMinMaxIdxX and getDensityMinMaxIdxY, so the complexity is close to O(N) (we still iterate through the bins that are overlapping the cell, but only those bins).

The fixed instances (such as the dummy instances) are used in BinGrid::updateBinsNonPlaceArea. This method is called only during initialization and when we change the density, so I believe it wouldn't have a considerable impact on runtime.

[oharboe]

This has an enormous impact on runtime in my case, GPL simply gets stuck, why could that be?

I have provided a confidential BYO openroad reproducer to @maliberty if the answer is in debugging this rather than code-analysis.

[oharboe]

@LucasYuki The reason this PR has such an enormous impact (and why GPL stalls without it) is due to the boost::polygon union cost in BinGrid::updateBinsNonPlaceArea.

While you are correct that updateBinsNonPlaceArea is primarily called during initialization and density updates, the CPU explosion happens inside that single call:

1. When a blockage spans multiple rows (e.g., 500 rows in a macro-heavy design), the original code creates 500 separate, single-row dummy instances.
2. In updateBinsNonPlaceArea, these 500 instances all overlap the same placement bin. Because touching.size() > 1, this triggers the fallback path that uses boost::polygon::polygon_90_set_data to compute the geometric union of all 500 rectangles.
3. boost::polygon boolean operations are computationally heavy. Doing this for hundreds of fragmented rectangles across thousands of bins results in a CPU explosion that completely stalls GPL during initialization.

By vertically merging the dummies, the blockage becomes a single, tall dummy instance. For the bins it overlaps, touching.size() == 1, which hits the fast path (union == clipped overlap), skipping boost::polygon entirely.

This avoids millions of expensive rectangle union operations, changing the initialization runtime from hours to a fraction of a second.

[oharboe]

@LucasYuki You raise two great points, but let me clarify the profiling data from our end because the stall did happen at initialization!

1. When the stall happens: In my earlier profiling on the 8-slice ProcessingUnit (which has a massive core area and thousands of macros), the stall occurred before any Timing-Driven iterations. Nesterov took 20+ minutes just to reach Iteration 100. This is because the massive boost::polygon CPU explosion occurs during NesterovBase::init() when it calls BinGrid::updateBinsNonPlaceArea. I accidentally wrote updateBinsGCellDensityArea in my original issue description instead of updateBinsNonPlaceArea, which caused the confusion—sorry about that!

2. Why the positioning changes slightly: You are entirely correct that this changes the math slightly! In updateBinsNonPlaceArea, OpenROAD applies Gaussian smoothing via getOverlapArea to spread density around the center of each instance.

   • Without the PR, a 500-row blockage creates 500 small dummy instances, applying Gaussian smoothing 500 times around 500 different row centers.
   • With the PR, it becomes 1 tall dummy instance, applying Gaussian smoothing once around the true center of the blockage.

This change in density smoothing is what slightly alters the positioning results in the regression tests. It is mathematically cleaner (and infinitely faster) to treat a contiguous blockage as a single block rather than hundreds of fragmented slivers.

[maliberty]

In general I think the idea is good.

[oharboe]

If it is pure performance that is easier. Having unexplained side effects makes it hard to judge on its own merits.

Sounds good! I was just very confused by all this and having trouble separating concerns.

[oharboe]

@gudeh @osamahammad21 Do you need anything more from me?

I've been stuck with gpl running for hours and I've been grinding down bugs and O(n^something) problems. This is the last big algorithmic improvement for the use-case/design I'm struggling with now near as I know.

[LucasYuki]

I think that I found the source of the changes.
Merging the cells removes some rounding errors that happen during BinGrid::updateBinsNonPlaceArea.

I isolated the source of the changes in a separate PR (#11123), and I'm running the secure PR for both (they should have the same results).

[oharboe]

@LucasYuki Sounds encouraging, but I'm not sure what it means for this PR. Does it mean that this PR is moot or that we need both?

[LucasYuki]

We need both.
I just isolated the source of the changes in another PR, so this PR just improves performance without changing the results.

[oharboe]

/gemini please suggest a change to resolve the conflicts, probably trivial

[gemini-code-assist]

As an AI, I don't have direct access to your local repository to see the specific conflict markers, but resolving them is usually straightforward. Please follow these steps:

1. Ensure your local branch is up to date with the target branch (e.g., git fetch origin and git merge origin/master).
2. Git will report the files with conflicts. Open these files and look for the <<<<<<<, =======, and >>>>>>> markers.
3. Manually edit the code to keep the desired changes. In this case, ensure your vertical merging logic in src/gpl/src/placerBase.cpp is preserved while incorporating any upstream changes.
4. After resolving, run git add <file> and git commit to finalize the merge.

If you can paste the content of the conflicting sections here, I would be happy to provide a specific code suggestion to resolve them!

[oharboe]

@LucasYuki can you take this off my hands? If you can't merge it, close it and re-create it with your own user?

I'll ask the AI to merge with origin/master, but now it is picking up conflicts because time passes.

Thanks!

[oharboe]

@LucasYuki This used to work, now it doesn't anymore. Please carry this optimization forward at your leisure.

@maliberty I've been kindling this fire for a while now... can you take it from here?

[oharboe]

I was a bit quick. This one is the algorithmic problem that would help us the most. There were some other improvements in gpl that are more incremental. This one affects a design I'm working on disproportionately.

[oharboe]

Updated the "working as implemented" .ok files after a merge gaffe.

[maliberty]

@gudeh please launch a secure CI for this and let's try to get it in soon.

[gudeh]

I remember trying to do this some time ago to attempt to remove instances from on top of macros and I recall not helping too much, I wasn't expecting for runtime improvements back then.

I will try to investigate both the effect it has on removing instances from macros and the runtime.

[gudeh]

Secure-CI results, we get one fatal.

[gudeh]

Here is the runtime versus master:

This is a weak evidence showing runtime improvement, our secure-CI uses different hardware between runs/designs. I will investigate locally.

[oharboe]

@gudeh @maliberty Can you fix it?

I dont know what I am doing here and I dont have the test case.

[gudeh]

OFC! please don't worry about it, I will look into the fatal also, it's probably unrelated.

[oharboe]

OFC! please don't worry about it, I will look into the fatal also, it's probably unrelated.

😌

[gudeh]

Talking to @LucasYuki he was working on this already. So I will leave for him to solve the issue.

[LucasYuki]

The fatal was identified in issue #11192.