Update OpenROAD: rsz buffer core-area clamping fix#4183
Open
openroad-ci wants to merge 10 commits intoThe-OpenROAD-Project:masterfrom
Open
Update OpenROAD: rsz buffer core-area clamping fix#4183openroad-ci wants to merge 10 commits intoThe-OpenROAD-Project:masterfrom
openroad-ci wants to merge 10 commits intoThe-OpenROAD-Project:masterfrom
Conversation
Update OpenROAD submodule to include fix for RSZ-inserted buffers landing outside core area, causing DPL legalization failures. OpenROAD PR: The-OpenROAD-Project-private/OpenROAD#3302 Signed-off-by: minjukim55 <mkim@precisioninno.com>
…oad-rsz-core-clamp Signed-off-by: minjukim55 <mkim@precisioninno.com> # Conflicts: # tools/OpenROAD
Point OR submodule at secure-rsz-core-clamp-only (ORFS master OR + squashed clamp commit only, without the naming refactors and dpl-negotiation-fixes that were bundled in the previous OR pointer). Rules threshold bumps for the 4 designs confirmed clamp-causal on the baseline experiment: - gf12/ca53 base: finish hold TNS -2170 -> -3080 - rapidus2hp/hercules_is_int base: GR+finish hold TNS -0.947 -> -1.5 - rapidus2hp/hercules_is_int verific: cts/GR/finish setup+hold loosened - rapidus2hp/hercules_idecode verific: cts setup TNS -0.392 -> -0.653 jpeg gf12 intentionally not bumped - expected to PASS now with the non-clamp OR commits removed (dpl-negotiation-fixes was the suspected cause of its deterministic -13235.9 / -8677.59 regression). Signed-off-by: minjukim55 <mkim@precisioninno.com>
Signed-off-by: minjukim55 <mkim@precisioninno.com>
Signed-off-by: minjukim55 <mkim@precisioninno.com>
Signed-off-by: minjukim55 <mkim@precisioninno.com>
Build The-OpenROAD-Project#4 on secure-test-clamp-only-with-rules (isolated OR = ORFS-master base + clamp only) surfaced new metric failures in designs that passed on PR The-OpenROAD-Project#1637's build The-OpenROAD-Project#5 (master+drift+clamp). Drift commits were masking these clamp victims; isolated-OR test exposes them. asap7/mock-alu (base): cts__timing__setup__tns -14100.0 -> -14400.0 (measured -14317.6) globalroute__timing__setup__tns -18100.0 -> -20800.0 (measured -20686.1) finish__timing__setup__tns -15700.0 -> -19100.0 (measured -18956.4) nangate45/jpeg (base): finish__timing__setup__ws -0.144 -> -0.152 (measured -0.151125) Margins chosen to slightly exceed measured (~0.5-1%) to absorb run-to-run variance observed between baseline #1 and The-OpenROAD-Project#2 on same SHA. Separately: nangate45/swerv hit GRT-0116 congestion error; not a rules-metric failure, needs different treatment. Signed-off-by: minjukim55 <mkim@precisioninno.com>
Build The-OpenROAD-Project#4 on secure-test-clamp-only-with-rules surfaced nangate45/swerv GRT-0116 routing congestion not present on pre-clamp baseline. Clamped port buffers concentrate local density; with util=65% and addon=0.25 the target density forces packing too tight for routing. Formula (flow/scripts/util.tcl:179): target_density = util + (1 - util) * addon + 0.01 Before: 0.65 + 0.35 * 0.25 + 0.01 = 0.7475 After: 0.65 + 0.35 * 0.20 + 0.01 = 0.7300 Modest reduction matching gf180/jpeg level. If still congested, drop further toward sky130hd/jpeg (0.15) or ihp-sg13g2/ibex (0.10). Signed-off-by: minjukim55 <mkim@precisioninno.com>
OR submodule pointer bump: 44712df461 -> 506b437ff2 The prior squashed clamp commit accidentally included two local CTest output files under src/rsz/test/Testing/Temporary/ (CTestCostData.txt and LastTest.log) that should never be tracked. Amended the clamp commit in OpenROAD repo to drop them and force-pushed to secure-rsz-instance-outside-core. Bumping the pointer here so ORFS tracks the clean commit. No functional change. Signed-off-by: minjukim55 <mkim@precisioninno.com>
…oad-rsz-core-clamp Signed-off-by: minjukim55 <mkim@precisioninno.com> # Conflicts: # flow/designs/rapidus2hp/hercules_is_int/rules-verific.json
![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 updates design rules and configuration parameters across several designs, including timing thresholds, area constraints, and instance counts. It also includes a subproject update for OpenROAD. A review comment highlights a potential inconsistency in the hercules_is_int verific rules, where the globalroute setup TNS was updated but the corresponding finish setup TNS was left unchanged, unlike other related metrics in the same file.
| }, | ||
| "globalroute__timing__setup__tns": { | ||
| "value": -1190.0, | ||
| "value": -1020.0, |
There was a problem hiding this comment.
The update to globalroute__timing__setup__tns to -1020.0 is inconsistent with finish__timing__setup__tns (line 71), which remains at -1190.0. Since these metrics were identical before and other timing pairs in this file (such as setup/hold WS) were updated in tandem, finish__timing__setup__tns should likely also be updated to -1020.0 to maintain consistency.
4 tasks
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
2 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
Update OpenROAD submodule to include the fix for RSZ-inserted buffers landing outside the core area, causing DPL legalization failures.
QoR accommodations for clamp-driven shifts
Clamping port buffers changes placement/routing characteristics; a few public designs now sit outside the pre-clamp thresholds. Rules and one density knob adjusted to match the new reality:
designs/nangate45/jpeg/rules-base.json:finish__timing__setup__ws-0.144 → -0.152 (measured -0.151)designs/asap7/mock-alu/rules-base.json: 3 setup-TNS thresholds relaxed (cts/globalroute/finish) to match measured regressions (~0.5-1% headroom)designs/nangate45/swerv/config.mk:PLACE_DENSITY_LB_ADDON0.25 → 0.20 to relieve GRT-0116 congestion caused by clamped port-buffer local densityCI methodology
CI runs against the isolated OR branch (ORFS-master's OR pointer + clamp only, no unrelated drift) so QoR deltas are attributable to the clamp alone.
Dependency
Type of Change
Impact
Verification
rszregression 195/195 passRelated Issues
Fixes The-OpenROAD-Project-private/OpenROAD-flow-scripts#1622