Skip to content

gpl: accumulate nonPlaceArea before multiplying density - #11123

Draft
LucasYuki wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:gpl-accumulate-nonPlaceArea
Draft

gpl: accumulate nonPlaceArea before multiplying density#11123
LucasYuki wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:gpl-accumulate-nonPlaceArea

Conversation

@LucasYuki

@LucasYuki LucasYuki commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Accumulate the non-place area before multiplying the density to reduce rounding errors.
This PR should generate the same results as #11085.

Type of Change

  • Bug fix

Impact

This slightly changes some test results.

Verification

  • [ X ] I have verified that the local build succeeds (./etc/Build.sh).
  • [ X ] I have run the relevant tests and they pass.
  • [ X ] My code follows the repository's formatting guidelines.
  • [ X ] I have signed my commits (DCO).

Related Issues

#11085

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors BinGrid::updateBinsNonPlaceArea in nesterovBase.cpp to accumulate raw non-place areas in a temporary vector before scaling and adding them to the bins, and updates getOverlapArea to return int64_t instead of float. The review feedback suggests wrapping this new accumulation logic in a check to ensure pb_->nonPlaceInsts() is not empty, which avoids unnecessary memory allocation and loop overhead when there are no non-placeable instances.

Comment on lines +927 to +945
std::vector<int64_t> nonPlaceAreaRaw(bins_.size(), 0);
for (auto& inst : pb_->nonPlaceInsts()) {
std::pair<int, int> pairX = getMinMaxIdxX(inst);
std::pair<int, int> pairY = getMinMaxIdxY(inst);
for (int y = pairY.first; y < pairY.second; y++) {
for (int x = pairX.first; x < pairX.second; x++) {
Bin& bin = bins_[y * binCntX_ + x];
bin.addNonPlaceArea(getOverlapArea(&bin, inst, dbu_per_micron)
* bin.getTargetDensity());
nonPlaceAreaRaw[y * binCntX_ + x]
+= getOverlapArea(&bin, inst, dbu_per_micron);
}
}
}
for (size_t i = 0; i < bins_.size(); ++i) {
if (nonPlaceAreaRaw[i] == 0) {
continue;
}
bins_[i].addNonPlaceArea(
static_cast<int64_t>(nonPlaceAreaRaw[i] * bins_[i].getTargetDensity()));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If pb_->nonPlaceInsts() is empty, allocating nonPlaceAreaRaw with the size of bins_ and then iterating over all bins is an unnecessary $O(\text{bins_})$ overhead. Wrapping this logic in a check to ensure pb_->nonPlaceInsts() is not empty avoids this allocation and loop entirely, keeping the complexity at $O(1)$ when there are no non-placeable instances.

  if (!pb_->nonPlaceInsts().empty()) {
    std::vector<int64_t> nonPlaceAreaRaw(bins_.size(), 0);
    for (auto& inst : pb_->nonPlaceInsts()) {
      std::pair<int, int> pairX = getMinMaxIdxX(inst);
      std::pair<int, int> pairY = getMinMaxIdxY(inst);
      for (int y = pairY.first; y < pairY.second; y++) {
        for (int x = pairX.first; x < pairX.second; x++) {
          Bin& bin = bins_[y * binCntX_ + x];
          nonPlaceAreaRaw[y * binCntX_ + x]
              += getOverlapArea(&bin, inst, dbu_per_micron);
        }
      }
    }
    for (size_t i = 0; i < bins_.size(); ++i) {
      if (nonPlaceAreaRaw[i] == 0) {
        continue;
      }
      bins_[i].addNonPlaceArea(
          static_cast<int64_t>(nonPlaceAreaRaw[i] * bins_[i].getTargetDensity()));
    }
  }

Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
Signed-off-by: LucasYuki <lucasyuki@yahoo.com.br>
@openroad-ci
openroad-ci force-pushed the gpl-accumulate-nonPlaceArea branch from 93a8da1 to 84e99b1 Compare August 18, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant