Skip to content

dpl: ensure site exists for one site gap check - #11101

Open
gudeh wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
gudeh:dpl-ensure-site
Open

dpl: ensure site exists for one site gap check#11101
gudeh wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
gudeh:dpl-ensure-site

Conversation

@gudeh

@gudeh gudeh commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Only checking if abut_pixel != nullptr does not guarantee a site is present, it must also be a valid site. Include this check to avoid false positive one site gap check for a macro on a private design.

Type of Change

  • Bug fix

Impact

no-op, fixes a corner case.

Verification

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

Related Issues

Replaced PR #11036, which would solve the issue by disregarding macros on one site gap. The present PR is better and more complete, making sure a site actually exists.

gudeh added 3 commits August 7, 2026 18:05
…or one site gaps

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…exists before checking for one site gap

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
@gudeh
gudeh requested a review from a team as a code owner August 7, 2026 20:26
@gudeh
gudeh requested a review from osamahammad21 August 7, 2026 20:26

@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 updates the placement check logic in CheckPlacement.cpp to ensure a site exists before checking for one-site gaps. The review feedback suggests simplifying the conditional logic to eliminate redundant null checks and improve readability.

Comment on lines +411 to +413
const bool site_exists = (abut_pixel && abut_pixel->is_valid);
const bool abuttment_exists = (abut_pixel && abut_pixel->cell);
if (!abuttment_exists) {
if (site_exists && !abuttment_exists) {

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

The temporary boolean variables site_exists and abuttment_exists are only used once in the immediate if condition. Additionally, they perform redundant null checks on abut_pixel. Simplifying this condition directly improves readability and maintainability.

        if (abut_pixel && abut_pixel->is_valid && !abut_pixel->cell) {

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