rsz: retire resizeToTargetSlew in fanout/hold/split-load repair (#8013)#10717
rsz: retire resizeToTargetSlew in fanout/hold/split-load repair (#8013)#10717saurav-fermions wants to merge 2 commits into
Conversation
…OpenROAD-Project#8013) Move the three remaining internal callers of the older resizeToTargetSlew sizing heuristic to the modern resizeToCapRatio path: - RepairDesign::makeRepeater (repeater resize-on-backup) - RepairHold::makeHoldDelay (hold buffer resize) - SplitLoadCandidate::resizeInsertedBuffer (split load move) To preserve the repair_design hot-path optimization, resizeToCapRatio gains an optional load_cap_hint parameter mirroring resizeToTargetSlew (skip ensureWireParasitic under global-routing parasitics). The RepairDesign site forwards its existing hint; the other two sites pass none. resizeToTargetSlew itself is kept as it still backs the public resizeDrvrToTargetSlew Tcl command, which is out of scope for The-OpenROAD-Project#8013. Regenerates 14 rsz goldens reflecting the small, bounded QoR delta of the buffer-sizing heuristic change (all violations still repaired; repair_slew13 improves a previously-violated slew). Baseline comparison confirms the churn is caused solely by this change. Full rsz regression passes 238/238. See AGENT_REPORT.md for the per-site analysis and golden-churn breakdown. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the legacy resizeToTargetSlew method with the modern resizeToCapRatio method across several files, including RepairDesign.cc, RepairHold.cc, and SplitLoadCandidate.cc. It also updates resizeToCapRatio to accept an optional load_cap_hint parameter, allowing it to skip expensive wire parasitic estimations when global routing parasitics are used. Feedback was provided regarding a potential issue where a load_cap_hint of 0.0 would evaluate to true, setting load_cap to 0.0 and silently preventing resizing. It is recommended to ensure the hint is greater than 0.0 before using it.
| if (load_cap_hint.has_value() | ||
| && estimate_parasitics_->getParasiticsSrc() | ||
| == est::ParasiticsSrc::kGlobalRouting) { | ||
| load_cap = *load_cap_hint; |
There was a problem hiding this comment.
If load_cap_hint is passed with a value of 0.0 (which is common for default or uninitialized capacitance hints in the caller), load_cap_hint.has_value() will evaluate to true. This will set load_cap to 0.0, bypassing the else block where the actual load capacitance is computed. Consequently, the subsequent check if (load_cap > 0.0) will fail, silently preventing any resizing from occurring.
To prevent this, we should ensure that the hint is a valid positive capacitance before using it, falling back to the computed load capacitance otherwise.
if (load_cap_hint.has_value() && *load_cap_hint > 0.0
&& estimate_parasitics_->getParasiticsSrc()
== est::ParasiticsSrc::kGlobalRouting) {
load_cap = *load_cap_hint;
|
@precisionmoon this looks like a good change but will need a secure CI - please launch one. |
A load_cap_hint of 0.0 satisfies has_value() and was used directly, making the later load_cap > 0.0 guard fail and silently skipping resizing. Require a positive hint in both resizeToTargetSlew and resizeToCapRatio, otherwise fall back to computing the load cap from parasitics. Addresses gemini-code-assist review on The-OpenROAD-Project#10717. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
|
Good catch — a |
| [INFO RSZ-0051] Resized 22 instances: 22 up, 0 up match, 0 down, 0 VT | ||
| [WARNING RSZ-0062] Unable to repair all setup violations. | ||
| No differences found. | ||
| Differences found at line 42. |
There was a problem hiding this comment.
You need to update split_load_hier_out.vok file also.
3 participants
Summary
Retires the remaining uses of the older
resizeToTargetSlewsizing heuristic in the fanout/hold/split-load repair call sites, moving them to the modernresizeToCapRatiosizing (as requested by the rsz maintainer in #8013).Type of Change
Impact
Unifies sizing on the modern path; affected rsz golden outputs updated accordingly.
Verification
ctest -R '^rsz\.'238/238.Related Issues
Fixes #8013
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).