rsz: Merge rsz-buffering-boost for Rebuffer QoR gain#10866
Conversation
Signed-off-by: Martin Povišer <povik@cutebit.org>
Recently repair_design received a new slew degradation model, use the same model in rebuffer. Signed-off-by: Martin Povišer <povik@cutebit.org>
r_drvr should not be scaled by the shape factor as it is derived from NLDM data as the asymptotic slew/load ratio and as such it already includes the shape factor. Signed-off-by: Martin Povišer <povik@cutebit.org>
When measuring the delay on a buffered net include the driving gate delay. Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
Fix load values passed to the gate delay calculator to be exclusive of the driving port capacitance. Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Minju Kim <mkim@precisioninno.com>
Signed-off-by: Minju Kim <mkim@precisioninno.com>
Cherry-picked from 86b8145b48 to fix STA-1350 gate delay load cap is NaN crash in fully_rebuffer when a net has zero wire RC (0 * inf). Signed-off-by: Martin Povišer <povik@cutebit.org> Signed-off-by: Minju Kim <mkim@precisioninno.com>
Regenerate unit .ok logs (rsz/gpl/openroad), their secondary golden outputs (.defok/.vok DEF/verilog), and aes/jpeg/ibex sky130 flow .metrics + .metrics_limits for the numeric churn from the estimation-accuracy rebuffering changes. The flow pass/fail gate is .metrics_limits (check_test_metrics compares the result against it; .metrics is the reference used to recompute the limits). Both are regenerated from a local run of the branch binary. Secondary DEF/verilog goldens are regenerated alongside the .ok logs so the in-test diff_file/diff_files comparisons pass and no "Differences found" text leaks into the committed .ok (No-Diffs-In-Ok-Files check). Note: ibex_sky130hs DRT::ANT::errors limit moves 0 -> 1 (one antenna violation from the changed routing). All other diffs are expected metric shifts (buffer counts, slew/cap slack, clock skew). No crashes; verified locally: rsz 241/241 pass, gpl tcl+py pass, openroad.upf_aes pass. Signed-off-by: Minju Kim <mkim@precisioninno.com>
|
Note The number of changes in this pull request is too large for Gemini Code Assist to generate a review. |
…g-boost Signed-off-by: Minju Kim <mkim@precisioninno.com> # Conflicts: # src/rsz/src/RepairDesign.cc
repair_design6 is a master-added test (reroute feature). After merging the rsz rebuffering estimation-accuracy series, the improved slew/RC estimation inserts 215 buffers (was 285) with slightly better slack (+115.5% vs +112.4%); no new violations. Regenerate the golden. Signed-off-by: Minju Kim <mkim@precisioninno.com>
Pin tools/OpenROAD at the rsz rebuffering estimation-accuracy series head (The-OpenROAD-Project/OpenROAD#10866, upstream author Martin Poviser). This drops the earlier SparseLU-guard bump so the submodule pin resolves against public OpenROAD for ORFS CI. Signed-off-by: Minju Kim <mkim@precisioninno.com>
gudeh
left a comment
There was a problem hiding this comment.
Changes to GPL are only golden test files.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17d1e1d0f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const float load_slew | ||
| = (size.driver_resistance | ||
| + resizer_->dbuToMeters(bnet->maxLoadWireLength() + extra_wire_length) | ||
| * wire_res) | ||
| * (bnet->cap() + extra_cap) * elmore_skew_factor_; | ||
| = size.driver_resistance * (bnet->cap() + segment_cap) | ||
| + segment_cap * segment_res * resizer_->slew_shape_factor_ / 2; |
There was a problem hiding this comment.
Include downstream cap in slew check
When extra_wire_length is nonzero, this check is deciding whether a buffer can drive the added wire segment, but it only charges the wire's own capacitance and omits the Elmore term for the downstream load (segment_res * bnet->cap()). addWire() and annotateTiming() propagate slew limits with wire_res * (wire_cap / 2 + p->cap()), so on long/high-cap fanout branches this underestimates the slew and can keep/export buffer options that still violate the sink max-slew budget.
Useful? React with 👍 / 👎.
| int rejected_move_count_ = 0; | ||
|
|
||
| // For Elmore slew modeling, see computeSlewShapeFactor() | ||
| float slew_shape_factor_ = 0.0; |
There was a problem hiding this comment.
Initialize slew factor before direct rebuffering
For standalone rsz::fully_rebuffer callers that do not run resizePreamble() first, this cached factor remains at 0 because Resizer::fullyRebuffer() only delegates to Rebuffer::fullyRebuffer() and Rebuffer::init() does not call computeSlewShapeFactor(). Existing direct flows such as src/rsz/test/rebuffer1.tcl call fully_rebuffer after parasitic estimation, so all new slew_shape_factor_ uses suppress wire/via slew degradation and can silently choose incorrect buffer trees; the previous lazy RepairDesign::getSlewRCFactor() path initialized the factor on demand.
Useful? React with 👍 / 👎.
| float a = wire_res * wire_cap * resizer_->slew_shape_factor_ / 2; | ||
| float b = (r_drvr * wire_cap) | ||
| + (wire_res * ref_cap * resizer_->slew_shape_factor_); | ||
| float c = (r_drvr * ref_cap) - max_load_slew_margined; |
There was a problem hiding this comment.
Solve the same slew equation used for the violation
When repairing a max-slew wire violation, this quadratic should solve the same load_slew expression used to enter and update the loop a few lines above, where the entire (r_drvr * ... + wire ...) sum is multiplied by resizer_->slew_shape_factor_. Leaving the driver terms unscaled here makes the computed split length correspond to a different equation whenever the shape factor is not 1, so repeaters can be placed too far from the load and the segment can remain over the max-slew budget or require extra repair iterations.
Useful? React with 👍 / 👎.
Signed-off-by: Minju Kim <mkim@precisioninno.com> # Conflicts: # test/aes_sky130hd.metrics # test/aes_sky130hd.metrics_limits # test/ibex_sky130hs.metrics # test/ibex_sky130hs.metrics_limits # test/jpeg_sky130hs.metrics # test/jpeg_sky130hs.metrics_limits # test/upf_aes.defok # test/upf_aes.ok
| double load_slew | ||
| = (r_drvr * (c_wire + ref_cap) + r_wire * ref_cap + r_wire * c_wire / 2) | ||
| * (*slew_rc_factor_); | ||
| * resizer_->slew_shape_factor_; |
There was a problem hiding this comment.
r_drvr considers the slew shape factor already. Multiplication by slew_shape_factor_ (double multiplication of slew_shape_factor_) looks incorrect.
There was a problem hiding this comment.
@jhkim-pii r_drvr ultimately comes from LibertyPort::driveResistance(). What makes you say it considers the shape factor already?
There was a problem hiding this comment.
Oops, I forgot how driveResistance is defined. So I agree, in effect it has the shape factor baked in and should not be multiplied by it.
| load_slew = (r_drvr * (c_wire + ref_cap) + r_wire * ref_cap | ||
| + r_wire * c_wire / 2) | ||
| * (*slew_rc_factor_); | ||
| * resizer_->slew_shape_factor_; |
| float r_drvr = resizer_->driveResistance(drvr_pin_); | ||
| assert(slew_rc_factor_.has_value()); | ||
| float load_slew = r_drvr * load_cap * (*slew_rc_factor_); | ||
| float load_slew = r_drvr * load_cap * resizer_->slew_shape_factor_; |
| double slew_left = r_drvr * cap_left * resizer_->slew_shape_factor_; | ||
| double slew_slack_left = maxSlewMargined(max_load_slew_left) - slew_left; | ||
| double slew_right = r_drvr * cap_right * (*slew_rc_factor_); | ||
| double slew_right = r_drvr * cap_right * resizer_->slew_shape_factor_; |
| const BnetPtr in2 = addWire(aux2, node->location(), -1); | ||
| odb::Point p1 = aux1->location(); | ||
| odb::Point p2 = aux2->location(); | ||
| odb::Point p3 = crit2->location(); |
There was a problem hiding this comment.
I think the third point should be node->location().
crit2 is not related to the new junction point of aux1, aux2, and node.
| Pin Limit Slew Slack | ||
| --------------------------------------------------------------- | ||
| _548_/B 320.000 317.570 2.430 (MET) | ||
| _326_/B 320.000 405.257 -85.257 (VIOLATED) |
There was a problem hiding this comment.
@minjukim55 I see this test where the violation has shown up has repair_design -reroute. Perhaps that matters
c340f92 to
b383f98
Compare
Integrate the rsz policy/optimizer refactor, reroute feature, and other master changes. The rebuffering estimation-accuracy series source is resolved against the refactored rsz; the affected test goldens are regenerated in the following commit. Signed-off-by: Minju Kim <mkim@precisioninno.com>
09ec48a to
20a3489
Compare
Regenerate unit .ok logs (rsz/gpl/openroad), their secondary golden outputs (.defok/.vok DEF/verilog), and aes/jpeg/ibex sky130 flow .metrics + .metrics_limits for the numeric churn from the estimation-accuracy rebuffering changes. The flow pass/fail gate is .metrics_limits (check_test_metrics compares the result against it; .metrics is the reference used to recompute the limits). Both are regenerated from a local run of the branch binary. Secondary DEF/verilog goldens are regenerated alongside the .ok logs so the in-test diff_file/diff_files comparisons pass and no "Differences found" text leaks into the committed .ok (No-Diffs-In-Ok-Files check). Also regenerate the repair_design6 golden (a master-added reroute test): the improved slew/RC estimation inserts fewer buffers with slightly better slack and no new violations. Note: ibex_sky130hs DRT::ANT::errors limit moves 0 -> 1 (one antenna violation from the changed routing). All other diffs are expected metric shifts (buffer counts, slew/cap slack, clock skew). Signed-off-by: Minju Kim <mkim@precisioninno.com>
20a3489 to
0cb1ec3
Compare
Advance the branch to current OR master so PR The-OpenROAD-Project#3585 stays mergeable (master moved 20d2d5c -> 06c1421: rsz max-iter/drive-resistance/cell-order, dpl negotiation, rcx). Golden conflicts (aes/jpeg sky130hd metrics) are resolved and regenerated against the merged binary in the following commit. Signed-off-by: Minju Kim <mkim@precisioninno.com>
After advancing to OR master 06c1421 (merge M2) and with the slew_shape_factor fix reverted, regenerate the flow goldens against the merged binary: - aes_sky130hd / ibex_sky130hs / jpeg_sky130hd: recompute .metrics_limits (DRV slew/cap slack + clock_skew shift from the reverted slew estimate). - tinyRocket_nangate45: reverting the fix restores the slew_shape_factor double-count, which over-estimates slew -> extra buffering -> global-route congestion overflow (GRT-0228). Enlarge the pipe-cleaner floorplan (die 500->540, core 490->530) so it routes; regen .metrics/.metrics_limits. Signed-off-by: Minju Kim <mkim@precisioninno.com>
Summary
Port of the upstream
rszrebuffering-accuracy series (original authorMartin Povišer). Core idea: rebuffer builds its buffer trees from an internal
delay/slew estimate that had drifted away from the model
repair_design(andsignoff STA) actually use — so rebuffer would optimize for numbers that later
stages disagree with. The series realigns that estimate and fixes several
errors in the buffered-net cost model.
The 8 substantive commits:
Resizer— relocate the slew "shape factor"(the NLDM-derived slew/load correction) into the shared
Resizersorebuffer and
repair_designread one consistent value instead of keepingseparate copies that can diverge.
repair_designrecently got a new slew degradation model; adopt the identical model inside
rebuffer so its internal estimate matches the signed-off timing model.
r_drvrby the shapefactor.
r_drvris the asymptotic slew/load ratio derived from NLDM dataand already contains the shape factor, so the extra scaling double-counted
it.
buffered net, include the driving gate delay so the measurement uses the
same delay definition as the estimator.
Steiner junctions of the rewritten buffer tree more accurately, tightening
the estimated wirelengths and loads the cost model sees.
exclusive of the driving port capacitance (the driver's own pin cap was
being counted into the load it drives).
length-estimation path so the iterative length estimate stays accurate on
long, high-fanout nets.
0 * inf→
NaN); fixes the STA-1350 "gate delay load cap is NaN" crash infully_rebufferon nets with no parasitics.(The branch also carries small housekeeping commits: two clang-format fixes and
one test-golden update for the numeric churn below.)
QoR
5 random placement seeds × 2 binaries (base = pre-series, test = this branch)
on a customer design (advanced-node PDK), full flow
place → CTS → route.Everything downstream of the binary is identical (same floorplan, same seeds),
so the delta isolates the change.
%= test vs base (+ = better).per-clock
fmaxmetric picks a different worst path and can move opposite toWS.
PLACE
CTS
GRT (global route)
Takeaway: consistent setup-WS / fmax improvement from CTS onward
(CTS +8.98%, GRT +7.08% fmax mean; 5/5 seeds improve at both stages). PLACE is
seed-noisy and nets to a small gain — the estimate matches the signed-off timing
model once CTS/routing make that model accurate.
Related Issues
Upstream
rszrebuffering-accuracy series. QoR validation tracked internally(private).