rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail by jhkim-pii · Pull Request #11803 · The-OpenROAD-Project/OpenROAD

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

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

Summary

Add SizeDownFanoutMt, the Mt form of fanout size-down, to the PARALLEL_EVAL WNS tail. Every serial move with an Mt form now runs in PARALLEL_EVAL.
Give every Mt generator the same steps, so each piece of code has one thread.

Changes

SizeDownFanoutMt measures one drive step down per off-path fanout load on the main thread, keeps the steps within the load's delay budget and the max-cap and slew limits, and scores the target path with the lighter load.
Mt generator steps: isApplicable() filters a target on the main thread without timing, the virtual prepareInputs(Target&) reads live STA on the main thread right before generation, and generate() and estimate() use only the prepared target on the workers. RebufferMt generate() is the one exception: its planner reads timing while it plans on a copy of the net.
PARALLEL_EVAL stops at its checkpoint when the walked design area reaches -max_utilization.
-sequence accepts size_down; without -sequence the WNS tail adds SizeDownFanoutMt, and -skip_size_down keeps it out.

flowchart LR
  D["critical driver"] --> C["critical load"]
  D --> F["off-path fanout load:<br/>one drive step smaller"]

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
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 (this PR)
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:43:41Z

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 the experimental PARALLEL_EVAL policy to repair setup timing in batches using multi-threading, replacing several older experimental policies. It adds comprehensive profiling capabilities, a work meter, and several new multi-threaded move generators (such as RebufferMt, SizeDownFanoutMt, SplitLoadMt, SwapPinsMt, UnbufferMt, and VtSwapMt). The code review identified several critical and high-severity issues, including potential null pointer dereferences in UnbufferMtGenerator.cc, RebufferPlannerPool.cc, SizeDownFanoutMtGenerator.cc, and SequentialTimingEstimator.cc that could lead to crashes, as well as a memory leak in DelayEstimatorBufferTree.cc from unreleased heap allocations.

jhkim-pii · 2026-10-17T05:44:01Z

@codex review

chatgpt-codex-connector · 2026-10-17T05:51:14Z

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

chatgpt-codex-connector · 2026-10-17T05:51:16Z

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:51:14.649699Z	`ce94857`	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.

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>

What: SwapPinsMt, the Mt form of the pin swap, joins the PARALLEL_EVAL WNS tail. Why: a pin swap is a cheap gain on the worst endpoints once the batches stall. How: Resizer::swapPinDelays() is shared with the serial move; the Mt generator measures the arc delays on the main thread and picks the fastest input on a worker. LEGACY is unchanged. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

What: UnbufferMt, the Mt form of buffer removal, joins the PARALLEL_EVAL WNS tail. Why: removing a buffer on the worst path is a gain the other Mt moves cannot make. How: the main thread checks the buffer and estimates the saved delay; the tail removes buffers only in single-move batches, since a removal cannot be reverted in place. LEGACY is unchanged. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

What: SizeDownFanoutMt, the Mt form of SizeDownFanout, joins the PARALLEL_EVAL WNS tail, and every Mt generator follows the same steps: isApplicable() and prepareInputs() on the main thread, generate() and estimate() on the workers. Why: the last serial move with an Mt form, and one rule for which code may read live STA. How: SizeDownFanoutMt measures the step-down cell per off-path load; the virtual MoveGenerator::prepareInputs(Target&) replaces the per-move prepare branches of the policy. PARALLEL_EVAL also stops at the walked -max_utilization area. LEGACY is unchanged. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

maliberty · 2026-10-18T02:30:30Z

Claude

== Priority 1 (correctness) ==

SizeDownFanoutMtGenerator.cc sortedFanoutSlacks()/measureLoads() question: the commit
message and the PR description say "per off-path fanout load", but nothing filters out
the load that is on the target path. estimateLoadChange() only models the lighter load
on the driver, using the current cell for later stages. So if the on-path load is
stepped down, its own slower gate is left out of the score, and the score is too
optimistic. (The serial move has the same loop, but there STA judges the commit.) Should
the critical load be skipped, or the estimate account for its new cell?
SetupParallelEvalPolicy.cc:1374 / :982 (tabu + batch_instances_) question: tabu and the
undo/exclude counters are keyed on target().inst(), which is the driver. A culprit
size-down therefore blocks the driver's other moves (sizeup, vt_swap) for N batches,
while the load it actually changed stays free to be stepped down again through another
driver. Is that intended?
SetupParallelEvalPolicy.cc recordSwap() note: kSizeDownFanout isn't recorded as a cell
swap, so salvage can't revert a culprit size-down in place; the whole batch rolls back.
That's safe, just a missed cheap revert. Extending SwapRecord to the touched (load)
instance would cover it.
SetupParallelEvalPolicy.cc finishAtMaxArea() question: the comment and the new test
say removeBuffer() doesn't update the area that overMaxArea() tracks. That is a Resizer
bookkeeping bug. LEGACY and the final report still read overMaxArea(), and
rollBackJournal() already works around it with recountDesignAreaAfterUndo(). Can
removeBuffer() be fixed at the source instead of walking the area in the policy?

== Priority 4-6 (architecture / style / process) ==

SizeDownFanoutMtGenerator.cc copies ~450 lines of SizeDownFanoutGenerator.cc almost
verbatim (resolveLoad, rankSwappableCells, the cap/slew checks, computeDelayBudget,
computeWorstDelayChange, the step-down choice). Please factor these into a shared
helper used by both, so LEGACY and PARALLEL_EVAL can't drift apart.

maliberty · 2026-10-18T02:30:44Z

Codex

Correctness — zero slack bypasses the delay budget. SizeDownFanoutMtGenerator.cc:639 rejects delay growth only when the budget is positive or negative. At exactly zero slack, any growth passes, so sizing down can create a new setup violation. Treat zero as having no growth budget.
Timing question — sequential loads lack an input-side check. SizeDownFanoutMtGenerator.cc:409 budgets a sequential cell swap from its output slack alone, while SizeDownFanoutMtCandidate.cc:35 scores only the lighter upstream load. Could the smaller sequential cell worsen its D-pin setup or hold check? The existing sequential swap estimator appears relevant here.

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-size-down-mt branch from b122ec2 to ce94857 Compare October 17, 2026 05:42

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

openroad-ci force-pushed the secure-rsz-size-down-mt branch from ce94857 to c7a85c5 Compare October 17, 2026 07:23

maliberty approved these changes Oct 18, 2026

View reviewed changes

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

maliberty deleted the secure-rsz-size-down-mt branch October 18, 2026 04:40

Navigation Menu

Repository navigation

rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail - #11803

rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail#11803
maliberty merged 7 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-rsz-size-down-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!

Uh oh!

jhkim-pii commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Uh oh!

maliberty commented Oct 18, 2026

Uh oh!

maliberty commented 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!

Uh oh!

jhkim-pii commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 17, 2026

Codex Review Summary

Uh oh!

maliberty commented Oct 18, 2026

Uh oh!

maliberty commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jhkim-pii commented Oct 17, 2026 •

edited

Loading