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

[saurav-fermions]

Summary

Hoist loop-invariant accessors out of the getDensityGradient inner loop in global placement. Placed DEF is bit-identical (MD5 unchanged).

Split out of #10849 as its own PR per review feedback (@gudeh) — that PR now contains only the detailed-placement (src/dpl) changes.

Testing

Built openroad; gpl regression suite passes, placed DEF bit-identical to baseline.

Notes

• Pure constant-factor optimization; no placement behavior change. No src/sta change.

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[saurav-fermions]

/gemini review

[gemini-code-assist]

Code Review

This pull request optimizes the NesterovBase::getDensityGradient function by hoisting loop-invariant values out of the inner loops to improve performance. The reviewer suggests using a const reference to the vector instead of calling .data() to avoid potential dangling pointer issues if the return type of getBinsConst() changes in the future.

[gemini-code-assist]

Using .data() on the result of bg_.getBinsConst() can be brittle. If getBinsConst() were to be changed to return by value in the future, this would create a dangling pointer.

A more robust approach is to hold a const reference to the vector. This makes the code safer against future refactoring of getBinsConst() and is just as performant.

  const auto& bins = bg_.getBinsConst();
  const float scale = gCell->getDensityScale();

  for (int i = pairX.first; i < pairX.second; i++) {
    for (int j = pairY.first; j < pairY.second; j++) {
      const Bin& bin = bins[j * binCntX + i];
      float overlapArea = getOverlapDensityArea(bin, gCell) * scale;

[gudeh]

I triggered a secure-CI with our designs to make sure this is no-op. I wonder if this would lead to any runtime improvements at all, I would guess not.

[gudeh]

Unnecessary comment

[gudeh]

Snake case for variables. I know gpl code has a lot to be changed, but we should use Google C++ style.

[saurav-fermions]

Thanks @gudeh — addressed:

• Unnecessary comment: removed.
• Snake case: binCntX → bin_cnt_x (the other two locals were already lower-case).
• Also took the gemini point on .data() being brittle — now holds a const auto& reference to getBinsConst() and indexes that, instead of caching a raw pointer.

gpl regression suite passes (69/69), placed DEF bit-identical.

On the runtime question: you're right to be skeptical — this is a micro-optimization (hoisting getBinCntX()/getDensityScale()/the bins accessor out of the inner loop over bins). It's meaningful only on high-bin-count designs where getDensityGradient is hot; on typical designs the win is small. It's purely a constant-factor cleanup with no QoR change, so if your secure-CI confirms it's a no-op on runtime for your designs, I'm fine if you'd rather not take it — no objection to closing.

[gudeh]

all the same at secure-CI, I am fine with merging this.

[gudeh]

Are you an AI? Which model are you?

[saurav-fermions]

Thanks for running the secure-CI and for the approval!

And a fair question — happy to be transparent. Yes, these are AI-assisted contributions. I'm the CTO of fermions.co (background: AI engineer working in the semiconductor domain), and we're building an agent called Saige that we're using to help produce these patches. Every contribution is reviewed, built, and tested by me before it's submitted, and I'm the accountable maintainer standing behind each PR — so if anything needs iteration or revert, that's on me.

Appreciate the careful reviews; they've been genuinely useful in tightening these up.