Skip to content

pdn: add basic handling of lef58 area rule#10488

Merged
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
gadfort:pdn-min-area
May 22, 2026
Merged

pdn: add basic handling of lef58 area rule#10488
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
gadfort:pdn-min-area

Conversation

@gadfort
Copy link
Copy Markdown
Contributor

@gadfort gadfort commented May 22, 2026

Summary

Adds checking of the LEF58_AREA rule.
Note, only handles the most basic case at this time.

Type of Change

  • Bug fix

Impact

Honors min area rules

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

[Link issues here]

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@gadfort gadfort requested review from a team as code owners May 22, 2026 00:53
@gadfort gadfort requested a review from maliberty May 22, 2026 00:53
Copy link
Copy Markdown
Contributor

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

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 introduces support for dbTechLayerAreaRule in the PDN generation process, specifically updating the adjustToMinArea function to account for these rules. The review feedback identifies critical issues in the implementation of the area calculation, including precision loss from using integer types for floating-point area values and incorrect scaling when converting from microns squared to DBU squared.

Comment thread src/pdn/src/techlayer.cpp
@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Copy Markdown
Member

@maliberty maliberty left a comment

Choose a reason for hiding this comment

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

The code looks fine. I'll wait for feedback from @jeffng-or to merge

@jeffng-or
Copy link
Copy Markdown
Contributor

Yeah, I think the code is doing what it's supposed to.

The issue I see is that the proprietary tech LEF min area constraint is small enough that the 4000 DBU per UU doesn't have enough granularity to represent the value correctly as an int. I'll file a separate issue for that one, but that shouldn't block this PR.

@jeffng-or
Copy link
Copy Markdown
Contributor

@gadfort
Copy link
Copy Markdown
Contributor Author

gadfort commented May 22, 2026

@jeffng-or it's worth noting that the database incorrectly stores the area as area (double) * DBU, when it should be area * DBU * DBU

@maliberty maliberty merged commit 67f0055 into The-OpenROAD-Project:master May 22, 2026
16 checks passed
@maliberty
Copy link
Copy Markdown
Member

@gadfort do you want to do a PR for the units?

@gadfort
Copy link
Copy Markdown
Contributor Author

gadfort commented May 22, 2026

@maliberty happy to since I actually had it, but will need it reviewed / check for how that change interacts with DRT

@gadfort gadfort deleted the pdn-min-area branch May 22, 2026 20:04
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.

3 participants