gpl: parallelize the CPU density scatter in place

[oharboe]

What

Parallelize the per-cell density scatter in BinGrid::updateBinsGCellDensityArea
on the CPU path, in place, using std::atomic_ref<int64_t> with
memory_order_relaxed on the existing int64_t bin accumulators. Bit-identical
to the previous serial loop at any thread count.

The CPU path is now a single scatter implementation,
BinGrid::scatterDensityAreaInPlace(), used at every thread count; the serial
duplicate is deleted.

That is the point rather than a side effect. -threads 1 is a diagnostic
configuration, not how any non-trivial design is placed -- ORFS passes
-threads $(NUM_CORES). If the serial and threaded cases ran different loops,
debugging a placement problem at -threads 1 would mean debugging code that
production never executes, and a bug that reproduces serially would not
necessarily be the bug you have. With one path, the diagnostic run exercises
exactly the production scatter, and so do all 104 gpl tests.

Why the CPU path was serial, and why it does not have to be

The parallel branch added for the GPU path accumulates into flat float
buffers. The comment there states the consequence:

The CPU-only path keeps the serial branch for bit-stable regression goldens.

That is true of that implementation: std::vector<float> accumulation makes
the total depend on thread arrival order.

But the bin accumulators are not floats. instPlacedAreaUnscaled_ and
fillerArea_ are int64_t, and each addend is truncated before it is added:

inline void Bin::addInstPlacedAreaUnscaled(int64_t area)
{
  instPlacedAreaUnscaled_ += area;
}

So the total is a sum over a fixed multiset of integers: associative and
commutative, hence independent of the order threads reach it. Accumulating in
place on those fields is therefore order-independent by construction, and
needs no flat buffers.

std::atomic_ref rather than #pragma omp atomic or an atomic member: the
field stays a plain int64_t, so Bin remains trivially copyable for its
std::vector and no other accessor changes, and memory_order_relaxed puts
the actual claim in the source -- atomicity is required, ordering is not, and
the accumulators are read only after the parallel region's implicit barrier.

The per-addend float value is unchanged too. The scatter computes a chain of
multiplies, (overlap * scale) * targetDensity; hoisting the conditional
targetDensity factor out preserves that left-to-right order, and with no
addition in the chain there is nothing for FP contraction to fuse either way.

Bin clear and the float density/overflow loop are left serial, so the change is
confined to the scatter. The flat-buffer path stays under ENABLE_GPU.

Performance

Measured on two in-tree gpl tests, so you can reproduce both from this repo
alone. 16 threads, global_placement only (DEF read/write excluded), baseline
and patched interleaved, median of 5 runs per arm on an otherwise idle machine.
Placement is unchanged: Final HPWL was a single distinct value across all ten
runs of each design.

test	design	utilization	baseline	patched	change
macro01	bp_be_top, 76,015 inst, has macros	12.3%	108,284 ms	62,326 ms	-42.4%
large01	netcard, 274,700 inst, std cells only	65.9%	107,768 ms	89,745 ms	-16.7%

Run-to-run spread was 106,409..123,011 / 58,598..65,067 for macro01 and
106,425..109,057 / 89,518..91,351 for large01; the distributions do not
overlap in either case.

Read the gap between those two rows before taking the first one. Same
machine, same thread count, same patch: 42% on one design and 17% on the other.
The gain is a property of the design, not of the change:

• -threads defaults to 1, and a default openroad invocation gets no
  speedup at all -- identical output, no parallelism. Only multi-threaded runs
  benefit; ORFS passes -threads $(NUM_CORES).
• It scales with bins x cells. Bin count goes as 1/placement density, so
  macro01 at 12.3% utilization has a far larger bin grid relative to its cell
  count than large01 at 65.9%, and the scatter is correspondingly more of its
  runtime. A dense design lands nearer the 17% end, a small one in the noise.
• Macros weight it further: their bin-overlap counts are an order of
  magnitude larger than a standard cell's.

Cost on the diagnostic path, since routing every run through the atomic is not
free: core01 at -threads 1, median of 9 interleaved runs, is 30 ms
baseline against 31 ms patched. That is the worst case available -- 294
instances over a 16x16 grid, so the bin array stays in L1 and the locked
read-modify-write has no cache-miss latency to hide behind. It matches the
arithmetic (~250k accumulations at ~20 cycles). I have no single-threaded
figure for the large designs -- the attempt was spoiled by an unrelated
workload, see the hygiene note in the recipe -- so I will only say that I expect
it to matter less there, since the scatter is then bound by misses on a
multi-megabyte bin array rather than by arithmetic. That is reasoning, not a
measurement.

For context on why the table is hedged rather than headlined: my earlier gpl
performance claims (#11073, #11072, #11086, #11084) were measured at a density
that flattered them, did not survive re-measurement, and are closed with
corrected numbers. Hence stock in-tree tests, medians over interleaved runs, and
the ranges printed above.

Reproduction recipe: measure it yourself

Three in-tree gpl tests span the relevant regimes:

test	design	instances	utilization	regime
macro01	bp_be_top	76,015	12.3%	low density, has macros -- most favourable
large01	netcard	274,700	65.9%	dense, std cells only
large02	leon3mp	312,529	63.6%	dense, std cells only

(large03 has no DEF checked in; it cannot run standalone.)

1. Build both binaries -- once at this branch's head, once at
origin/master -- keeping each openroad aside as openroad-patched /
openroad-base.

2. Time global placement only, excluding DEF read/write. For each test,
derive a driver from the test's own Tcl so the setup matches exactly:

# time-macro01.tcl -- generated from src/gpl/test/macro01.tcl
read_lef ./nangate45.lef
read_lef ./bp_be_top_macro.lef
read_def ./macro01.def
set t0 [clock milliseconds]
global_placement -density 0.7
set t1 [clock milliseconds]
puts "GP_MS [expr {$t1 - $t0}]"
source report_hpwl.tcl

Run from src/gpl/test (the Tcl uses relative paths):

openroad-base    -no_init -threads 16 -exit time-macro01.tcl
openroad-patched -no_init -threads 16 -exit time-macro01.tcl

3. Benchmark hygiene -- this is where I lost a day. An unrelated job on the
same box moved my baseline from 97 s to 175 s, and later from 122 s to 496 s, on
identical input. taskset pinning did not rescue it, because the foreign job
was unpinned and floated onto the same cores. So:

• Verify the machine is idle for the whole run, not just at the start. Check
  for stray openroad processes before and after every single run and discard
  any run that was not alone; "idle at launch" is what produced the 4x spread
  above.
• Interleave the arms (base, patched, base, patched, ...) rather than all of one
  then all of the other, so drift cannot masquerade as an effect.
• At least 5 reps per arm; report the median and the full spread. If the
  baseline arm's spread is comparable to the difference you are claiming, you
  have measured the machine, not the patch.
• Sanity check: Final HPWL must be identical between arms. If it is not,
  something other than this patch is in play.

4. Vary the thread count (-threads 1,2,4,8,16) on macro01. Scaling with
thread count is the signal that you are measuring this loop; a flat curve means
the scatter is not your bottleneck on that design.

Determinism evidence

In the test suite. New mt_invariance01 runs core01 at
set_thread_count 8 and diffs against core01's own single-threaded golden.
That golden is the full log, including the entire 237-iteration
overflow/HPWL/penalty trace, so it pins the whole trajectory rather than a final
number. Registered in both CMake and Bazel. This is the coverage that was
missing: the rest of the suite runs at the default -threads 1.

Full suite. 104/104 pass (bazelisk test --nocache_test_results //src/gpl/...).

At scale, by hand. macro01, large01 and large02 each run four ways --
baseline at 1 and 16 threads, patched at 1 and 16 threads -- with the four logs
compared byte for byte. Identical in every case, modulo the ORD-0030
thread-count banner. Final HPWL: 4665013.08, 5128184.78, 4483580.32.

Open questions

The ENABLE_GPU branch is now the only user of the flat-buffer path. I do not
build that configuration, so I have left it untouched rather than claim the two
can be collapsed -- with #10831 adding a Kokkos/CUDA build that is not a
formality. If the integer-accumulator argument holds there too, the float
buffers and their per-iteration allocation can go away entirely, but that is for
someone who can run it.

Two things I noticed in this function and did not touch, but which you may
want to know about:

• BinGrid::setNumThreads() has no callers, so BinGrid::num_threads_ is
  always 1 and the loops using it are silently serial.
• One of those is sumOverflowArea_ += overflowArea under reduction(+:),
  where sumOverflowArea_ is int64_t and overflowArea is float. That
  converts the running total to float on every add, so unlike the scatter it
  genuinely is thread-order-dependent -- the scatter is safe precisely because
  each addend is truncated to an integer before accumulation, which this is
  not. It is latent only because the thread count above is pinned to 1. Anyone
  repairing the dead setNumThreads() would make global placement
  nondeterministic without touching a line of placement code.

Separately, #11123 touches nearby code in this file. Happy to rebase behind it,
or to hand this over, whichever is less disruptive.

[gemini-code-assist]

Code Review

This pull request parallelizes the CPU path for updating bin grid cell density areas using OpenMP. It introduces atomic addition methods to safely update bin areas across multiple threads. The reviewer noted that using atomic operations even during single-threaded execution introduces unnecessary performance overhead due to atomic instructions. They suggested branching on the thread count to run a non-atomic loop when running serially, and using explicit casts to avoid compiler warnings.

[oharboe]

Good catch on the atomics, and it was worse than the comment suggests: the
if (parallel_threads > 1) clause suppressed only the threading, so the
#pragma omp atomic still executed at -threads 1. The PR body claimed the
default path was unchanged, which was wrong. Fixed, along with everything else
I could find by re-reading the patch adversarially rather than waiting for it
to come in one finding at a time.

Rather than branch into an atomic and a non-atomic loop, I went the other way:
there is now one scatter implementation,
BinGrid::scatterDensityAreaInPlace(), used at every thread count, and the
serial duplicate is deleted (net removal of the 43-line loop). The reason is
testability -- with two paths, 103 of the 104 gpl tests exercised the serial
loop and nothing exercised the code threaded runs actually take. Now they all
do. The tradeoff is that a single-threaded run pays an uncontended atomic in
this hotspot; I took one code path over a serial fast path deliberately. If you
would rather keep the fast path, template <bool kAtomic> on that single
function with if constexpr at the two call sites restores it without
duplicating the loop body, and I am happy to do that instead -- it is your
call, not mine.

Also changed:

• std::atomic_ref<int64_t> with memory_order_relaxed instead of the
  OpenMP pragma. The repo is C++20, so it is available. The field stays a plain
  int64_t, so Bin remains trivially copyable for its std::vector and no
  other accessor changes; and the explicit memory order states the actual
  claim, that atomicity is required and ordering is not. An std::atomic<>
  member would have been worse -- it puts an atomic RMW on every serial add
  too.
• Explicit static_cast<int64_t> at both call sites. The per-addend
  truncation is the whole correctness argument, so it should be visible rather
  than implicit.
• mt_invariance01, registered in both CMake and Bazel: runs core01 at
  set_thread_count 8 and diffs against core01's single-threaded golden --
  the full log, including the entire 237-iteration overflow/HPWL/penalty trace.
  Verified byte-identical.
• Dropped a comment that claimed the CPU path had to stay serial for
  bit-stable goldens.

On the float-to-int question specifically: the ordering argument does not rely
on FP contraction being off. The scatter is a chain of multiplies with no
addition in it, so there is nothing to fuse; hoisting the conditional
targetDensity factor preserves the original left-to-right order exactly.

Two things I noticed in this function and deliberately did not touch:

1. BinGrid::setNumThreads() has no callers, so BinGrid::num_threads_ is
   always 1 and the loops using it are silently serial.
2. One of those is sumOverflowArea_ += overflowArea under reduction(+:),
   with int64_t on the left and float on the right. That converts the
   running total to float on every add, so it genuinely is
   thread-order-dependent -- unlike the scatter, which is safe precisely
   because each addend is truncated to an integer before accumulation.
   Latent only because the thread count is pinned to 1. Worth knowing before
   someone repairs the dead setNumThreads() and makes global placement
   nondeterministic without touching any placement code.

Still no performance figure in the PR, by choice -- the reasoning and a
reproduction recipe are in the description.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a multi-threaded, order-independent density scatter implementation (scatterDensityAreaInPlace) on the CPU path using integer accumulators to ensure bit-identical results across different thread counts. It also adds a new multi-threading invariance test. The reviewer feedback suggests replacing std::atomic_ref with #pragma omp atomic for better portability and C++20 independence. Additionally, it is recommended to template scatterDensityAreaInPlace on a bool kAtomic parameter to avoid atomic instruction overhead during single-threaded execution.

[oharboe]

Thanks — but I'm declining all five of these, because they optimise the one
axis I'm deliberately not optimising. On this code, maintainability beats
single-threaded speed: a serial fast path is not worth a second copy of the
hotspot to keep correct.

On templating scatterDensityAreaInPlace<bool kAtomic> (three of the five).
This is the option I offered in my previous comment, so I want to be clear it
is a real option and I'm turning it down for reasons beyond taste:

• It undoes the change's main testing benefit. With one path, all 104 gpl tests
  exercise the code that threaded runs take. With <true>/<false>, the
  default-thread suite exercises <false> and only mt_invariance01 ever
  instantiates <true> — putting the concurrent path back behind a single
  test, which is where this started.
• It adds a footgun the current code cannot express. In the suggested form the
  #pragma omp parallel for num_threads(parallel_threads) stays unconditional,
  so scatterDensityAreaInPlace<false>(cells, 8) compiles and silently races.
  Today no call can race, because there is only one accumulate and it is
  atomic. Making a data race reachable to save an uncontended lock xadd is a
  bad trade in a function that has already had one determinism-related comment
  go stale.
• Two instantiations of a ~30-line hotspot is exactly the duplication the
  single path removed.

On reverting std::atomic_ref to #pragma omp atomic (the other two). The
stated benefits don't apply here:

• "does not require C++20" — this repo already requires it:
  set(CMAKE_CXX_STANDARD 20) in the top-level CMakeLists.txt and
  -std=c++20 in .bazelrc. There is nothing to gain.
• "degrades to a non-atomic addition when OpenMP is disabled" — that is only
  safe because the enclosing parallel for disappears at the same time. It is
  an accident of two pragmas vanishing together, not a property I want the
  correctness of a density accumulator to rest on. find_package(OpenMP REQUIRED) in src/gpl/CMakeLists.txt means the configuration cannot arise
  anyway.
• "avoids atomic overhead in single-threaded builds" — this is the trade I'm
  declining, stated plainly.

What atomic_ref buys that the pragma does not: memory_order_relaxed is
written down, so the claim being made — atomicity required, ordering not — is
in the source instead of in a reviewer's head; and it is visible to
sanitizers and clang-tidy, which OpenMP atomics are not.

One thing in there that is worth taking seriously, though not for the reason
given: std::atomic_ref needs libstdc++ 10+ / libc++ 19+, while this repo's
CMakeLists.txt still gates at gcc >= 8.3 and AppleClang >= 12. Those gates
predate the C++20 requirement and are inconsistent with it, but Mac-Build is
the one job that would actually catch a libc++ without atomic_ref. I'm
waiting on it. If it fails, I'll switch these two accessors back to
#pragma omp atomic — on portability grounds, keeping the single code path
either way, rather than reintroducing a serial fast path.

[oharboe]

One more argument against the templated <bool kAtomic> split, which I think
is the decisive one and which I under-sold above.

No non-trivial design is placed on a single thread. -threads 1 is
predominantly a diagnostic configuration: it is what you reach for to get a
reproducible run, to bisect a placement difference, or to attach a debugger
without thread interleaving in the way. Production is -threads $(NUM_CORES),
which is what ORFS passes.

That makes divergence between the serial and threaded scatter actively harmful,
rather than merely untidy. With <false> for serial and <true> for threaded:

• Debugging at -threads 1 would execute a loop that production never runs. A
  bug that reproduces serially is then not necessarily the bug you have, and
  one that does not reproduce serially tells you nothing about whether it is a
  concurrency bug or a different code path.
• The 104 gpl tests would all cover <false>; only mt_invariance01 would
  instantiate <true>. The configuration nobody ships gets the test coverage,
  and the one everybody ships gets one test.

So the diagnostic use case is not an argument for a serial fast path -- it is
the strongest argument against one. The point of running single-threaded is to
observe the code that runs multi-threaded, and that only works if they are the
same code.

Which reframes the performance question too: the atomic on the serial path
costs something I have not measured, but it is a cost paid by a diagnostic mode
and by designs small enough not to care. That is the wrong axis to optimise,
and it is not worth a second copy of the hotspot to keep correct.

The std::atomic_ref portability question stands separately on its own merits;
I am still waiting on Mac-Build, and if it fails I will move those two
accessors back to #pragma omp atomic while keeping the single code path.

[oharboe]

The machine finally went idle, so this PR now has measured numbers instead of a
recipe and an apology. Both from in-tree gpl tests, reproducible from this repo
alone -- 16 threads, global_placement only, interleaved, median of 5 per arm,
nothing else running:

test	utilization	baseline	patched	change
macro01	12.3%, has macros	108,284 ms	62,326 ms	-42.4%
large01	65.9%, std cells only	107,768 ms	89,745 ms	-16.7%

Placement is unchanged: Final HPWL was a single distinct value across all ten
runs of each design. The distributions do not overlap on either -- worst-case
patched still beats best-case baseline by 39% on macro01.

The 42/17 split is the part worth reading. Same machine, same thread count,
same patch; the gain belongs to the design, not to the change, and scales with
bins x cells (bin count goes as 1/density). A dense design sits nearer 17%, a
small one in the noise, and a default -threads 1 run gets nothing at all.

Also measured, since I had asserted it earlier without evidence: the
single-threaded cost of routing every run through the atomic accumulate is
30 ms -> 31 ms on core01, median of 9. That is the worst case available --
294 instances over a 16x16 grid, so the bin array stays in L1 and the locked
read-modify-write has no cache-miss latency to hide behind. ~1 ms on a 30 ms
placement, and it matches the arithmetic (~250k accumulations at ~20 cycles).
I had written that cost into the description as a caveat before measuring it;
it is a rounding error in a diagnostic mode, and it does not change the case
against a second code path.

Everything green: 16/16 checks including Mac-Build, which also settles the
std::atomic_ref availability question I raised against my own change.