fix: wrong pin in loadCap during hold repair#10092
Merged
eder-matheus merged 1 commit intoThe-OpenROAD-Project:masterfrom Apr 11, 2026
Merged
fix: wrong pin in loadCap during hold repair#10092eder-matheus merged 1 commit intoThe-OpenROAD-Project:masterfrom
eder-matheus merged 1 commit intoThe-OpenROAD-Project:masterfrom
Conversation
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
2 times, most recently
from
36137ad to
3175d6f
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
Author
|
Hi @eder-matheus @maliberty, |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
maliberty
approved these changes
Apr 9, 2026
Member
maliberty
left a comment
maliberty
left a comment
There was a problem hiding this comment.
@eder-matheus please run a secure CI
@precisionmoon FYI
Member
|
sky130he/aes will need a metrics update |
Member
|
Secure-CI started. |
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
2 times, most recently
from
2ea6484 to
c5f1102
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
2 similar comments
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
from
c5f1102 to
71e719d
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
from
71e719d to
02a4fd2
Compare
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
from
02a4fd2 to
d7e69f9
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
In repairEndHold(), the buffer delay guard used loadCap() on the wrong pin (end_vertex, a sink) to estimate the inserted buffer's load capacitance. This returned near-zero cap, making the guard trivially permissive and relying entirely on the post-insertion journal rollback. Replacing it with loadCap() on the driver pin (path_vertex) is semantically more correct but overestimates the buffer load: it includes the full pre-split net parasitics, which are absent on the shorter post-split net. This causes buffer_delays to be overestimated, valid hold buffer insertions to be rejected by the guard, and consequently worse slew and capacitance slack. Fix: compute load_cap directly as the sum of pin capacitances of load_pins — the exact set of pins the inserted buffer will drive. This avoids counting parasitics from net segments that will not exist after splitting, giving an accurate and non-pessimistic guard estimate. Signed-off-by: Suhrid Marwah <suhridmarwah07@gmail.com>
suhr25
force-pushed
the
fix/repair-hold-load-cap
branch
from
d7e69f9 to
efe551e
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
Author
|
All CI's are passing now. |
Member
|
@eder-matheus did private pass? |
Member
Yes, all private designs are passing. Only nangate45/swerv_wrapper failed, but I believe this is due to the Yosys non-determinism. This should be safe to merge. |
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
3 participants
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.
SUMMARY
This PR fixes a wrong pin being used in hold repair, which caused incorrect load capacitance calculation and unsafe buffer insertion. The issue is in
RepairHold::repairEndHold()(src/rsz/src/RepairHold.cc) and could introduce setup violations during hold fixing.FIX
Before
After
VERIFICATION
Running
repair_timing -holdbefore this fix could insert buffers on paths without enough setup slack, causing new violations. After the fix, capacitance is computed correctly, so buffer insertion respects setup constraints and avoids breaking clean paths.