Rearchitect resizer repair setup

[openroad-ci]

Summary

• Rearchitect repair_setup around Optimizer, setup policies, move generators, and move candidates.
• Add target preparation/prewarm flow, MoveCommitter, repair target collection, delay estimation helpers, and shared utl::ThreadPool support.
• Preserve default legacy setup behavior while adding experimental setup policies (legacy_mt, mt1, measured_vt_swap).
• 20 Apr 2026: The default legacy setup policy shows identical ORFS design QoR with the master. But it may have corner-case QoR impact in other designs.
• Refer to the architecture documentation in src/rsz/doc/architecture.md.

TODO

1. Architecture
   a. Apply review feedback
   b. Consider moving MoveTracker out of MoveCommitter for detailed tracking
2. Fix UI
   a. Envar or command option
   b. Merge optimization policies (new) and phases (old)
3. Enhance setup optimization QoR and runtime
   a. Enhance VtSwap & SizeUp moves w/ MT
   b. Develop a score metric for fair comparison among different moves
   c. Ensure thread safety of APIs
   d. Better delay estimation
   e. Better optimization policy
4. Support DRC / hold / power optimization
5. Global optimization (e.g., Lagrangian relaxation)

Testing

• Experimental legacy_mt policy regression: ctest -R legacy_mt
• Experimental mt1 policy regression: ctest -R mt1
• cmake --build build --target TestResizerMt -j8
• ctest --test-dir build --output-on-failure -j8
  • 100% tests passed, 0 tests failed out of 7721
• Disabled long-running rsz tests (for faster rsz testing)
  • rsz.repair_design3.tcl
  • rsz.repair_design3_verbose.tcl
  • rsz.repair_fanout6.tcl

[gemini-code-assist]

Code Review

This pull request re-architects the repair_setup optimization flow into a policy-driven framework, replacing monolithic move classes with MoveGenerator and MoveCandidate pairs. It introduces a central Optimizer and various OptPolicy implementations, including experimental multi-threaded versions. The changes also include new stages for cache warming and data preparation to ensure thread safety. Feedback was provided regarding outdated documentation in Resizer.hh that incorrectly attributes move accounting to journaling methods instead of the new MoveCommitter class.

[gemini-code-assist]

The comment here seems to be outdated. The journal* methods in Resizer.cc now just call the eco* methods and do not handle move counting themselves. The move accounting logic has been moved to the MoveCommitter class. To avoid confusion, this comment should be updated to reflect that these are wrappers and that move counting is handled elsewhere.

References

1. When analyzing code within a loop, consider the entire loop structure. Operations at the beginning of the loop (e.g., journalBegin) can affect the logic within the loop body, potentially making similar calls inside the loop redundant.

[erendn]

I made some initial comments. There can also be the CostDeltas struct that @QuantamHD had mentioned. It can help set the scoring criteria based on the current policy. We need to have some heuristics for each move to estimate delay/power/area impact.

[erendn]

I agree. We can score candidates for hold/power/area and have separate policies for those.

[erendn]

It makes sense to have this Target expose endpoint and driver paths.

[erendn]

I think we can also have policies like:
CommonViolatorsPolicy: Collect violators based on how frequently they are encountered along violating paths and try to repair them.
BurstUpdatePolicy: Try multiple moves at a time and update STA together. If the target metric improves, commit all of them. Otherwise, revert and try those moves one by one.
GreedyUpsizePolicy: Do a greedy upsizing on violating gates, possibly pre-placement.

[jhkim-pii]

Yeah. Sounds good.
We can make new policies easily by adding new OptPolicy classes.

In the legacy resizer, it already has multiple phases (e.g., WNS, TNS, STARTPOINT_FANOUT, ENDPOINT_FANIN, LAST_GASP, ...), which is conceptually the same as policies.
I think we can split the legacy phases out of the big SetupLegacyPolicy class and provide flexible mixing of old and new policies.

[erendn]

Why doesn't MoveCandidate have isApplicable() instead?

[jhkim-pii]

This is the current logic.

• MoveGenerator has isApplicable() method. Necessary filtering should be done at the generate stage.
• MoveGenerator should create applicable MoveCandidates. But some of the MoveCandidates can be found to be illegal (e.g., DRC violation, ...) after estimate().

[erendn]

I didn't understand what isApplicable() does exactly. Does it check if the target has a certain view mask the move requires? Can we move that function to that move's candidate class?

[jhkim-pii]

isApplicable() is the place to apply various filters.

• Target view validation check is one of them.
• If a certain move supports post-CTS stage only, we can add such stage filter in that API.
• If we move isApplicable() to MoveCandidate, multiple MoveCandidates may do the same filtering. So having it in MoveGenerator looks better to me. Please let me know if you have a case that MoveCandidate::isApplicable() is better than MoveGenerator::isApplicable(). Having the both APIs will make users confusing.

[erendn]

What does it mean to "prepare targets"?

[jhkim-pii]

Prepare is a stage to cache necessary data to reduce redundant computation during estimation by multiple worker threads.
prepareTargets() annotates additional cached data for the efficient move candidate estimation.
Different move types may require different data to be cached.

But basically, this is burden caused by inconvenience of the process of retrieving state/data retrieval from STA in multi-threading environment.
If STA provides MT-friendly APIs for the data retrieval, Prepare stage can be eliminated in the future.

src/rsz/doc/architecture.md has an explanation about Prepare stage.

[erendn]

Do we really need a separate MoveGenerator for each move type?

[jhkim-pii]

I don't understand what you mean.
Do you think that a single MoveGenerator should know how to make all MoveCandidates of different move types?

In the current architecture, MoveGenerator and MoveCandidate are tightly-coupled.
I think this makes adding a new move more difficult because two new tightly-coupled classes should be created.

Currently, there are many generator and candidate classes for multiple move types.
The number of moves can be reduced by

• Once a new move supporting MT is developed, the old ST (single-thread) move should be retired.
• Merging similar moves. E.g.,
  • VtSwap, SizeUpMove and SizeUpMatchMove -> SizeMove (option control to enable Vt swap up-sizing & down-sizing the target)
  • SizeDownMove -> ReduceLoadMove // Because it reduces loads of the target instead of the target itself.
  • BufferMove and SplitLoadMove (subset of BufferMove) -> BufferMove

[erendn]

My question is also related to MoveGenerator::isApplicable(). What if we have only one MoveGenerator class and MoveCandidate classes have the isApplicable() function instead? It felt a bit redundant to have two classes for each move type.

And I agree that we should rename SizeDownMove to ReduceLoadMove and then we can implement a SizeDownMove that actually only downsizes the target.

[jhkim-pii]

It felt a bit redundant to have two classes for each move type.

Yeah. I don't like the two mandatory classes for each move type.

But let's imagine that one big MoveGenerator generates MoveCandidates for all move types. I think it will significantly hurt the code readability. Having multiple MoveGenerators would provide better readability.

[maliberty]

This split is directly from @QuantamHD original proposal. I think separating generation from result doesn't imply redundancy as they two classes have different responsibilities.

[erendn]

We can add new move types:
RelocateMove: Move a gate closer to its violating fanout gates
DecomposeMove: Break a complex logic gate into smaller gates
InvBufferMove: Replace a buffer with two inverters
VTSwapLeakageMove: Reduce VT of a non-violating gate

[jhkim-pii]

VTSwapLeakageMove: Reduce VT of a non-violating gate

I think a sizing move for power recovery can include this.

[erendn]

I think we can also have different violator collection modes:
PathMode: Collect the gates along the violating path
FaninMode: Collect the fanin cone of an endpoint
FanoutMode: Collect the fanout cone of a startpoint
IntersectionMode: Collect the common violators that intersect multiple violating paths

[jhkim-pii]

I made some initial comments. There can also be the CostDeltas struct that @QuantamHD had mentioned. It can help set the scoring criteria based on the current policy. We need to have some heuristics for each move to estimate delay/power/area impact.

Definitely, we need it.
Actually, I made an early version of the score/cost structure. But I deleted it from the PR because it is premature.
I found that score/cost should be designed tightly coupled with concrete MoveCandidate classes to provide a way to compare the scores from different move types fairly.
Developing a fair cross-move score metric looks not trivial to me.
So I am going to make a MT-buffering move and find a good score/cost struct for the fair comparison.
If you have a good idea, let's discuss about it. I don't have a good one yet except for a rough one (slack enhancement may be a good score for the fair cross-move comparison).

e.g.,

struct Estimate {
   bool legal{false};                // Feasible and DRC clean
   float score;                      // Output of a scoring equation (configurable) w/ the following PPA metrics.
                                     // Used for MoveCandidate sorting
                                     // Per-move-type adjustment may be required for fair cross-move comparison

   // Timing
   float delay_impr{0.0f};           // Delay improvement of the focused arc/path. Manage the rise/fall sense data or use sum/avg?
   float slack_impr{0.0f};            // Slack improvement may not be the same as slack improvement.
   float side_path_delay_impr{0.0f};    // Sum of delay deltas of the side-paths
   float side_path_slack_impr{0.0f};     // Sum of slack deltas of the side-paths

   // Power
   float total_pwr_impr{0.0f};      // Total power improvement
   float leak_pwr_impr{0.0f};       // Leakage power improvement
   float dyn_pwr_impr{0.0f};        // Dynamic power improvement.

   // Area
   float area_impr{0.0f};
}

The attributes can be more (mainly for analysis) or less than this.

• I think this should be aligned with the actual move implementation.
• The side-path impact is one of the crucial factors. But we don't have good delay/timing estimator for side-path impact yet.

[jhkim-pii]

@mguthaus @abk-openroad @ZhiangWang033 FYI

[mguthaus]

@erendn

[maliberty]

For scoring I think we should separate estimation of fields (like area, power, timing) from the combining to a single score. I think the policy should do the combining as different policies might have different goals (e.g. power recover vs setup fixing). The policy can tell the move estimator what fields it needs to avoid unnecessary computation.

[erendn]

We can build multiple SplitLoadCandidate here where each one has a different split (not necessarily 50/50).

[jhkim-pii]

Yes. It is an easy way to make the SplitLoadMove* support MT.

BTW, it looks like SplitLoad move is conceptually a subset of Buffer move. Then it may be better to make BufferGenerator generates load-splitting candidates together and remove the SplitLoad move because it is a subset of buffering.

[maliberty]

I think they have different goals. Split load is about separating off non-critical loads where buffering is about speeding up critical loads.

[povik]

@maliberty one of the ways generic buffering speeds up critical loads is by separating off non-critical loads so I agree with Jaehyun's "it looks like SplitLoad move is conceptually a subset of Buffer move."

To some degree the buffering code today already separates off loads (in what it calls "topology rewriting") and it might be smarter about it than SplitLoad, i.e. it does not use a fixed 50/50 split and considers placement. We can test if keeping SplitLoad is providing a benefit, perhaps we can retire it

[maliberty]

So long as the unified approach can achieve the same transformations / QOR that's fine.

[erendn]

SetupLegacyPolicy allows worsening timing to get out of local minima. Maybe we should leave the "legality" to be decided by each policy based on estimated metrics.

[jhkim-pii]

Right.
The current code is an initial draft and needs further improvement through research.
As a fundamental infra for optimization, more feature enhancements are needed especially for the delay/slack estimation to predict the timing impact more accurately over a wider range.

[jhkim-pii]

@erendn @maliberty I updated the followings.

Changes

1. UI changes:

• repair_timing phase selection was cleaned up by keeping -phases as the public interface, accepting -policy / -policies as aliases with conflict checking, updating the default setup-phase sequence to include CRIT_VT_SWAP, and warning when experimental setup policies are selected.
• Files: src/rsz/src/policy/OptimizationPolicy.cc/hh, src/rsz/src/policy/*   // Each phase is a separate policy now.

2. Delay estimator updates:

• The sizing delay estimator and its diagnostic reporter were refined. Now it estimates 3-stage delay instead of the target cell delay (old).
• Files: src/rsz/src/DelayEstimator.cc, src/rsz/src/DelayEstimator.hh, src/rsz/src/DelayEstimatorReporter.cc, src/rsz/src/DelayEstimatorReporter.hh, src/rsz/src/OptimizerTypes.cc, src/rsz/src/OptimizerTypes.hh

3. Other changes:

• repair_setup internals were further reorganized around dedicated phase-policy classes with a shared run context and simpler sequencing
• The resizer architecture document was updated accordingly
• Several rsz regressions were re-enabled or shortened to balance coverage and runtime.