rsz: Add SplitLoadMt to PARALLEL_EVAL by jhkim-pii · Pull Request #11800 · The-OpenROAD-Project/OpenROAD

jhkim-pii · 2026-10-17T04:31:43Z

Part 7 of 12 of the PARALLEL_EVAL setup repair stack. It builds on #11799 and also shows the commits of parts 1 to 6; its own change is the last commit.

Summary

Add SplitLoadMt, the Mt form of SplitLoad, to the PARALLEL_EVAL default sequence.

Changes

DelayEstimator::estimateLoadChange() scores a lighter target load over the same path window, and the moved loads are charged like fanin side loads.
SplitLoadMt makes the choice of the serial move on the main thread, the less critical half of the loads behind a buffer, and scores it with weight 0.5 because the model ignores the wire change.
-skip_buffering removes SplitLoadMt together with RebufferMt.

flowchart LR
  subgraph B["PARALLEL_EVAL default batches"]
    V["VtSwapMt"] --> S["SizeUpMt"] --> R["RebufferMt"] --> SL["SplitLoadMt<br/>(this PR)"]
  end
  subgraph T["WNS tail adds"]
    SP["SwapPinsMt"] --> U["UnbufferMt"] --> SD["SizeDownFanoutMt"]
  end
  B --> T

Stack

test: Add LEGACY repair setup path regressions #11794 test: Add LEGACY repair setup path regressions
rsz: Remove MT1, MEASURED_VT_SWAP, LEGACY_MT and the RSZ_* knobs #11795 rsz: Remove MT1, MEASURED_VT_SWAP, LEGACY_MT and the RSZ_* knobs
rsz: Add the setup repair profiler #11796 rsz: Add the setup repair profiler
rsz: Model side paths in the Mt delay estimator #11797 rsz: Model side paths in the Mt delay estimator
rsz: Add the RebufferMt buffer-tree move #11798 rsz: Add the RebufferMt buffer-tree move
rsz: Add the experimental PARALLEL_EVAL setup repair policy #11799 rsz: Add the experimental PARALLEL_EVAL setup repair policy
rsz: Add SplitLoadMt to PARALLEL_EVAL #11800 rsz: Add SplitLoadMt to PARALLEL_EVAL (this PR)
rsz: Add SwapPinsMt to the PARALLEL_EVAL WNS tail #11801 rsz: Add SwapPinsMt to the PARALLEL_EVAL WNS tail
rsz: Add UnbufferMt to the PARALLEL_EVAL WNS tail #11802 rsz: Add UnbufferMt to the PARALLEL_EVAL WNS tail
rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail #11803 rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail
rsz: Add the per-move analysis and the rebuffer accuracy report #11804 rsz: Add the per-move analysis and the rebuffer accuracy report
doc: Update the rsz setup repair architecture #11805 doc: Update the rsz setup repair architecture

github-actions · 2026-10-17T04:31:59Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

gemini-code-assist

Code Review

This pull request introduces a parallelized setup-repair policy, 'PARALLEL_EVAL', which utilizes worker threads for candidate generation and estimation, along with a new profiler for setup repair. It also includes significant refactoring of the delay estimator to support thread-safe timing estimation, introduces a bottleneck analysis for batch prioritization, and adds a thread-safe buffer tree planner. My review identified several critical issues, including potential null pointer dereferences in the new SplitLoadMtCandidate and SplitLoadMtGenerator code, exception safety concerns in RebufferPlannerPool, and potential cache misses due to floating-point bit-casting of negative zero. I have provided actionable feedback for these issues.

jhkim-pii · 2026-10-17T05:43:46Z

@codex review

chatgpt-codex-connector · 2026-10-17T05:43:59Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-17T05:50:58.295623Z	`967f688`	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-10-17T05:50:57Z

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Failed to sample tokens

ℹ️ 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".

What: the Mt delay estimator models the fanin side loads, the opposite output transition, the latch D-side driver and the flip-flop setup and hold change of a cell swap, and SizeUpMt and VtSwapMt charge them. Why: scoring a swap on the selected path alone let a scored gain turn into a loss after the timing update. How: side models in DelayEstimator and SequentialTimingEstimator, input cap budgets that workers can check without STA, and ArcDelayState renamed to EstimatorContext. The side models are off by default. Setup repair profiler review follow-ups: the profiler constructor takes a RepairSetupProfiler::Mode instead of an int, CRIT_VT_SWAP samples WNS and TNS before its sweep, and the Bazel TestResizerMt target drops the --gtest_filter that skipped the profiler and fixture tests. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

What: RebufferMt, an Mt buffering move that plans a beam of Rebuffer trees per driver and scores each with the buffer-tree estimator. Why: buffering needs the same scored, comparable candidates as the cell swaps before a batch policy can choose among them. How: makeRebufferTreePlans() and applyRebufferTreePlan() in Rebuffer, estimateBufferTreeWithSidePaths() in DelayEstimator, and current required times after the endpoint scan. No policy runs RebufferMt yet; PARALLEL_EVAL will. LEGACY is unchanged. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

What: PARALLEL_EVAL, an experimental setup repair policy that commits a batch of VtSwapMt, SizeUpMt and RebufferMt moves per timing update. Why: one timing update per batch instead of per move cuts the repair time. How: worker threads scan endpoints and generate and score candidates from prepared data; BatchSelector, BottleneckAnalysis and ParallelEvalState pick, rank and stop; each batch is kept, salvaged or rolled back by its WNS and TNS (the group's under -path_group). OptimizationPolicy::finish() ends each phase, and the Optimizer prints the final report after the last phase. LEGACY results do not change. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

What: SplitLoadMt, the Mt form of SplitLoad, joins the PARALLEL_EVAL default sequence. Why: splitting off the less critical loads is a strong setup move that PARALLEL_EVAL lacked. How: estimateLoadChange() scores the lighter target load and the moved loads are charged as side loads; the split itself is chosen as in the serial move. LEGACY is unchanged. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

maliberty · 2026-10-18T01:14:52Z

Claude

== Priority 1 (correctness) ==

src/rsz/src/move/SplitLoadMtGenerator.cc:805 / SplitLoadMtCandidate.cc:365
question: the estimate scores findTargetCell(buffer_cell, cap), which
matches on target load, but apply() resizes with resizeToTargetSlew(),
which matches on target slew. These can pick different cells, so the
scored buffer delay and target_load_delta (from that cell's input cap) may
not match what gets committed. Can prepare call the same sizing function
that apply() uses?
SplitLoadMtCandidate.cc:296-301 question: the buffer delay uses
stage.current_slew (the driver's slew before the split) and ignores the
wire delay to the moved loads. Is that intentional? The new net can be
long, because moved_box is a HPWL over half the fanout.
SplitLoadMtGenerator.cc:632 vs 715 nit: the loads are ranked by
rf-specific slack (from the driver path transition), but each moved load's
side budget uses slack(vertex, min_max), which is the worst over both
rise and fall. The two disagree when the critical rf differs.
SplitLoadMtCandidate.cc:365-368 nit: bufferOutputPin() is evaluated again
after the resize. It works by port name, but computing the pin once before
resizing is clearer.

== Priority 2 (QoR) ==

repair_setup_parallel_eval_default_sequence.ok iter 9: the one split
leaves WNS unchanged (-15.861) and makes TNS 3.4x worse (-24.3 → -83.0,
viol endpoints 4 → 17), yet the batch is accepted. From iter 10 on, the
run converges to exactly the same WNS/TNS as before, with +1 buffer and
+0.1% area. On this test the split adds area for zero gain, and the side
loss charge did not catch the TNS hit. Is the moved-load penalty
underestimated (old input slew, no wire delay, weight 0.5)?
repair_setup_parallel_eval_path_group.ok: no split is applied (RSZ-0040,
not RSZ-0045), but the result changes: 22 → 26 buffers, 116 → 104
resizes, WNS -0.040 → -0.034. What changes the outcome when no split
commits? Possibly candidates conflicting in the shared pool, or the
impact() write sets blocking other moves. Worth understanding before
attributing gains to this move.
question: what are the ORFS results across platforms with and without
split in the default sequence? The PR shows no QoR data for making this a
default.

maliberty · 2026-10-18T01:17:02Z

+
+namespace {
+
+constexpr int kSplitLoadMinFanout = 8;


jhkim-pii self-assigned this Oct 17, 2026

github-actions Bot added the size/XL label Oct 17, 2026

gemini-code-assist Bot reviewed Oct 17, 2026

View reviewed changes

jhkim-pii marked this pull request as ready for review October 17, 2026 05:17

jhkim-pii requested review from a team as code owners October 17, 2026 05:17

jhkim-pii requested a review from precisionmoon October 17, 2026 05:17

openroad-ci force-pushed the secure-rsz-split-load-mt branch from 12a98ec to 967f688 Compare October 17, 2026 05:32

jhkim-pii added 4 commits October 16, 2026 23:51

openroad-ci force-pushed the secure-rsz-split-load-mt branch from 967f688 to 04a2b1b Compare October 17, 2026 07:13

maliberty approved these changes Oct 18, 2026

View reviewed changes

Comment thread src/rsz/src/move/SplitLoadMtGenerator.cc

namespace {

constexpr int kSplitLoadMinFanout = 8;

maliberty Oct 18, 2026

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 8?

maliberty merged commit 59d4379 into The-OpenROAD-Project:master Oct 18, 2026
20 checks passed

openroad-ci deleted the secure-rsz-split-load-mt branch October 18, 2026 01:34

Navigation Menu

Repository navigation

rsz: Add SplitLoadMt to PARALLEL_EVAL - #11800

rsz: Add SplitLoadMt to PARALLEL_EVAL#11800
maliberty merged 4 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-rsz-split-load-mt

jhkim-pii commented Oct 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhkim-pii commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Uh oh!

maliberty commented Oct 18, 2026

Uh oh!

maliberty Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jhkim-pii commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Stack

Uh oh!

github-actions Bot commented Oct 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhkim-pii commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Uh oh!

maliberty commented Oct 18, 2026

Uh oh!

maliberty Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jhkim-pii commented Oct 17, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 17, 2026 •

edited

Loading