pdn: test a ring layer named by -pins - #11210
Merged
gadfort merged 1 commit intoAug 23, 2026
Merged
Conversation
Grid::checkSetup() validates a -pins layer against a set built from the grid rings and its straps. Every existing case that uses -pins names a strap layer, so the straps_ loop alone satisfies the check and the rings_ loop is uncovered: removing it leaves the pdn regression green. This is core_grid_with_rings_with_straps with -pins metal6 added, the upper layer of its ring, so that case is the control and the two goldens differ only where the option acts: two block terminals with two ports each, and unchanged SPECIALNETS geometry. Confirmed intended behaviour in The-OpenROAD-Project#11209. Signed-off-by: shivarammysore <shivaram.mysore@gmail.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test case, core_grid_ring_on_pin_layer, to verify that a layer occupied only by a ring is a valid -pins layer and that the ring's segments correctly become block pins. The test is added to both the Bazel (BUILD) and CMake (CMakeLists.txt) build configurations, along with its expected output (.ok) and TCL script (.tcl). No review comments were provided, so there is no feedback to address.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the case from #11209, which @gadfort confirmed is intended behaviour ("yes rings can become pins so that is correct") and offered to review.
core_grid_ring_on_pin_layercovers therings_half of the-pinsvalidation inGrid::checkSetup():All fifteen existing cases that use
-pinsname a strap layer, so thestraps_loop alonesatisfies the check for every one of them — removing the
rings_loop leaves the pdn regressiongreen, and
-pins <ring layer>would then raise PDN-0111 with nothing to catch it.The case is
core_grid_with_rings_with_strapswith one option added,-pins metal6, the upperlayer of its ring. ⟹ its control is already in the suite: the existing case is the same design
without the option, so the two goldens differ only where the option acts — two block terminals,
two ports each (the ring's two metal6 sides), and unchanged SPECIALNETS geometry.
It reuses the design and LEF the existing case already reads, so it adds no new library data.
Registered in both
src/pdn/test/CMakeLists.txtandsrc/pdn/test/BUILD.The
.defokand.okwere generated at945a9f48dc6e5cc91d865daa92c45a1094cb682c(master at thetime of writing) and the case reports
No differences found.there. Happy to regenerate if CIdisagrees.
On
-max_rows/-max_columns: understood, leaving that for the CPP CI suite.