gpl/dpl: placement performance (bit-identical) by saurav-fermions · Pull Request #10849 · The-OpenROAD-Project/OpenROAD

saurav-fermions · 2026-07-09T07:09:45Z

Summary

Five constant-factor optimizations to global (src/gpl) and detailed (src/dpl) placement. Placed DEF is bit-identical (MD5 unchanged).

dpl — reuse diamondSearch scratch buffers (cut allocator churn); cut super-linear netlist-construction cost; cache master/site in checkPixels (congested-legalization); replace diamondSearch visited set with generation stamps.
gpl — hoist loop-invariant accessors out of the getDensityGradient inner loop.

Testing

Built openroad; full gpl + dpl regression suites pass (188/188, incl. golden log_compare). Placed DEF bit-identical to baseline.

Notes

Pure constant-factor optimizations; no placement behavior change.
Two further placement patches from the same internal set are intentionally excluded here: an FFT-threading change (conflicts with the current FftBackend refactor and is only QoR-equivalent) and a default-OFF Nesterov convergence mode (unmet prerequisite). Those can follow separately.
No src/sta change; no cross-repo dependency.

Opendp::diamondSearch() is called once per placed cell (hundreds of thousands of times in detailed placement) and allocated a fresh priority_queue backing vector and unordered_set<GridPt> on every call. Callgrind attributed ~13.5% of dpl cost to libc malloc/free and ~3.85% to hashtable inserts/rehashing from this churn. Reuse cleared, grown-once thread_local scratch buffers instead of reallocating per call. A small priority_queue subclass exposes its backing container so the allocation can be moved back into the reusable buffer on every return path. Buffers are function-local static thread_local (not members) because diamondSearch is const and could be called concurrently; this keeps reuse data-race free with no behavior change. Isolated dpl benchmark: median 1434 ms -> 1307 ms (8.9% faster). Placement DEF is byte-identical (MD5 unchanged); dpl regression suite 117/117 green. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>

gemini-code-assist

Code Review

This pull request introduces several performance optimizations to reduce memory allocations and ODB traversals during placement and legalization. Key changes include caching and reusing scratch buffers in diamondSearch, lazily materializing names for cells and edges, caching routing-layer bitmasks, and hoisting loop-invariant lookups. The reviewer suggests further optimizing the instance sorting in createNetwork by sorting by unique integer IDs instead of string names, which would completely eliminate string allocations and comparisons.

gemini-code-assist · 2026-07-09T07:12:12Z

+  std::vector<std::pair<std::string, dbInst*>> insts_by_name;
+  insts_by_name.reserve(block_insts.size());
+  for (dbInst* inst : block_insts) {
+    insts_by_name.emplace_back(inst->getName(), inst);
+  }
+  // Reserve the node/edge containers up front (cheap, exact counts) to avoid
+  // repeated reallocation of the unique_ptr vectors as the netlist is built.
+  network_->reserve(block_insts.size() + block->getBTerms().size(),
+                    block->getNets().size());
+  std::ranges::stable_sort(insts_by_name, [](const auto& a, const auto& b) {
+    return a.first < b.first;
+  });

-  for (dbInst* inst : insts) {
+  for (const auto& [inst_name, inst] : insts_by_name) {


Since sorting by string names is highly expensive, we should prefer sorting by unique integer IDs (e.g., a->getId() < b->getId()) to achieve a deterministic order. This avoids all std::string allocations, auxiliary vectors, and expensive strcmp calls entirely.

std::vector<dbInst*> insts(block_insts.begin(), block_insts.end()); // Reserve the node/edge containers up front (cheap, exact counts) to avoid // repeated reallocation of the unique_ptr vectors as the netlist is built. network_->reserve(block_insts.size() + block->getBTerms().size(), block->getNets().size()); std::ranges::stable_sort(insts, [](dbInst* a, dbInst* b) { return a->getId() < b->getId(); }); for (dbInst* inst : insts) {

References

For tie-breaking in sort comparators, prefer using unique integer IDs (e.g., cell1->id() < cell2->id()) instead of string comparisons (e.g., cell1->name() < cell2->name()), as string comparisons are highly expensive.

gudeh · 2026-07-09T15:09:43Z


  FloatPoint electroForce;

+  // Hoist loop-invariant values out of the inner loop. These do not change


This should be an independent PR

gudeh · 2026-07-09T15:10:49Z

  const bool all_ok = edge_ok && padding_ok && blocked_ok && gap_ok;

  if (!all_ok && logger_->debugCheck(DPL, "checkDRC", 1)) {
+    // Computing the cell name allocates a std::string; checkDRC is on the


unnecessary comment

gudeh · 2026-07-09T15:13:06Z

+// inside diamondSearch) so the scratch backing buffers can be declared as
+// reusable static thread_local containers below, avoiding a fresh heap
+// allocation on every call.
+struct PQ_entry


The negotiation legalizer will be the default one soon. The diamond search will be kept only for fallback on some corner cases (preferably never used).

Profiling the timed detailed_placement showed the diamondSearch BFS does NOT expand on already-globally-placed designs (avg ~1 candidate site examined per cell on large01, ~5 on medium03), so a spatial free-site index has no search to accelerate. The real super-linear term is in createNetwork(): it scaled 4.1x (271 -> 1114 ms) for a 2.8x cell increase (medium03 -> large01), dominated by per-pin / per-net work. Optimizations (all correctness-preserving, placement byte-identical): - Cache the routing-layer bitmask per dbMTerm instead of walking every pin's geometry for every iterm. The layer set depends only on the master's mterm geometry, shared across all instances of a master. Same bits are set as before, just deduplicated. - Derive edge names lazily from the backing dbNet rather than allocating net->getName() per net up front (getEdgeName has no eager callers). - Sort instances on cached name keys (O(N) string allocations) instead of calling dbInst::getName() inside the comparator (O(N log N) allocations); ordering is byte-identical. - Reserve node/edge containers from exact db counts. - Stop allocating cell->name() unconditionally on the hot checkDRC path. Measured (median warm runs, DEF read excluded from the timed region): - medium03 (~99K cells): 1406 -> 1408 ms (~0%, within noise) - large01 (~275K cells): 3574 -> 3381 ms (~5.4%) The win grows with design size, the signature of removing a super-linear term. Correctness: dpl regression suite 117/117 green; placement byte-identical to baseline (verified by diffing written DEF against a baseline rebuild); check_placement passes with 0 failures on medium03 and large01. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>

…bit-identical) On congested designs diamondSearch expands over many grid points, calling canBePlaced -> checkPixels per probe. checkPixels re-fetched cell-invariant DB handles on every probe via odb traversal: - cell->getSite() (dbInst->getMaster()->getSite()) - cell->getDbInst()->getMaster() (symmetry check) perf on a 100K-cell congested case showed odb::dbInst::getMaster() at ~7.6% self time, the top DB hot spot after diamondSearch itself. The dpl Master object already caches the odb::dbMaster*; cache its odb::dbSite* too (populated in setDbMaster) and read both cached pointers in checkPixels instead of walking odb. The cached pointers are identical to what the odb traversal returns for CELL nodes (Master is built from inst->getMaster()), so placement is unchanged. This is a constant-factor win, not an exponent change: it removes per-probe overhead that scales with probe count. Measured dpl-only speedup on congested Nangate45 scramble designs (min of 6 interleaved A/B runs): 50K: 900 -> 821 ms (8.8%) 100K: 1695 -> 1579 ms (6.8%) 200K: 4943 -> 4527 ms (8.4%) log-log scaling exponent ~1.23 unchanged (constant-factor, as expected). Bit-identical verified: - placed DEF MD5 match base vs opt on 50K/100K/200K congested designs - dpl ctest 118/118 green (.defok diff = byte-identical on all test designs) Signed-off-by: Saurav Singh <saurav.singh@fermions.co>

Opendp::diamondSearch is called once per placed cell (hundreds of thousands of times on large designs). It used a per-call std::unordered_set<GridPt> to dedup grid points pushed into the search priority queue; on congested designs the per-point malloc/free + hash churn dominated allocator cost. Replace it with a reusable thread_local generation-stamped flat array (VisitedStamps): a point is visited iff its stamp equals the current generation, and beginSearch() bumps the generation instead of clearing, so there is no per-call reallocation. Pure data-structure swap -- the search loop is unchanged. The stamp array is sized over {center} U [x_min,x_max]x[y_min,y_max] expanded by a one-cell halo, because contains() is probed before the limit test and the unconditionally-inserted center may lie outside the clipped window. This covers the reachable grid-edge coordinates (x_max/y_max and their +1 neighbors) that an earlier attempt missed, which had caused an infinite loop / OOM and dpl edge-test failures. Verified: 117/117 dpl ctest pass (incl. all prior edge failures); legalized DEF byte-identical to baseline on 50K and 100K congested designs; ~19-29% DPL speedup (1.24x-1.42x) measured at 50K/100K/200K. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>

saurav-fermions · 2026-07-09T22:35:10Z

Thanks @gudeh — addressed:

gpl change is now its own PR (gpl: hoist loop-invariant accessors in getDensityGradient (bit-identical) #10863). This PR is now detailed-placement (src/dpl) only.
Unnecessary comment in PlacementDRC.cpp: removed.
Place.cpp / negotiation legalizer: understood. The diamondSearch scratch-buffer reuse here is a pure allocation optimization and stays valid while diamond search is used (even as a fallback); happy to revisit once the negotiation legalizer is the default.
Accidental src/sta bump: removed (force-pushed).

On the gemini suggestion to sort by getId() instead of name: the name sort is pre-existing behavior — the original code did stable_sort(..., a->getName() < b->getName()). My change only caches each name once (to avoid O(N log N) getName() calls) and sorts on the cached key, preserving the exact original order so the result stays bit-identical. Sorting by id would change iteration order and break that guarantee, so I've kept the name sort.

saurav-fermions requested review from a team as code owners July 9, 2026 07:09

saurav-fermions requested review from gudeh and precisionmoon July 9, 2026 07:09

github-actions Bot added the size/M label Jul 9, 2026

gemini-code-assist Bot reviewed Jul 9, 2026

View reviewed changes

gudeh requested changes Jul 9, 2026

View reviewed changes

saurav-fermions force-pushed the feat/place-perf branch from 90c793e to 45375d4 Compare July 9, 2026 22:06

saurav-fermions added 3 commits July 9, 2026 22:10

saurav-fermions force-pushed the feat/place-perf branch from 45375d4 to 74dab5a Compare July 9, 2026 22:10

saurav-fermions mentioned this pull request Jul 9, 2026

gpl: hoist loop-invariant accessors in getDensityGradient (bit-identical) #10863

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

gpl/dpl: placement performance (bit-identical)#10849

gpl/dpl: placement performance (bit-identical)#10849
saurav-fermions wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
Fermions-ASI:feat/place-perf

saurav-fermions commented Jul 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 9, 2026

Uh oh!

gudeh Jul 9, 2026

Uh oh!

gudeh Jul 9, 2026

Uh oh!

gudeh Jul 9, 2026

Uh oh!

saurav-fermions commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		FloatPoint electroForce;

		// Hoist loop-invariant values out of the inner loop. These do not change

Uh oh!

Conversation

saurav-fermions commented Jul 9, 2026

Summary

Testing

Notes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

gudeh Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

gudeh Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

gudeh Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

saurav-fermions commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants