rsz: Add UnbufferMt to the PARALLEL_EVAL WNS tail

[jhkim-pii]

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

Summary

• Add UnbufferMt, the Mt form of buffer removal, to the PARALLEL_EVAL WNS tail.

Changes

• UnbufferMt checks on the main thread that a buffer on the target path may go and estimates the delay its removal saves; the removal is offered only when the estimate is a gain, and the batch timing update judges the real effect.
• The WNS tail removes buffers only in single-move batches, because a removal cannot be reverted in place inside the batch journal.
• -sequence accepts unbuffer; without -sequence the WNS tail adds UnbufferMt, and -skip_buffer_removal keeps it out.

flowchart LR
  D["driver"] --> B["buffer on the target path"] --> L["loads"]
  D -. "after removal" .-> L

Loading

Stack

1. test: Add LEGACY repair setup path regressions #11794 test: Add LEGACY repair setup path regressions
2. 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
3. rsz: Add the setup repair profiler #11796 rsz: Add the setup repair profiler
4. rsz: Model side paths in the Mt delay estimator #11797 rsz: Model side paths in the Mt delay estimator
5. rsz: Add the RebufferMt buffer-tree move #11798 rsz: Add the RebufferMt buffer-tree move
6. rsz: Add the experimental PARALLEL_EVAL setup repair policy #11799 rsz: Add the experimental PARALLEL_EVAL setup repair policy
7. rsz: Add SplitLoadMt to PARALLEL_EVAL #11800 rsz: Add SplitLoadMt to PARALLEL_EVAL
8. rsz: Add SwapPinsMt to the PARALLEL_EVAL WNS tail #11801 rsz: Add SwapPinsMt to the PARALLEL_EVAL WNS tail
9. rsz: Add UnbufferMt to the PARALLEL_EVAL WNS tail #11802 rsz: Add UnbufferMt to the PARALLEL_EVAL WNS tail (this PR)
10. rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail #11803 rsz: Add SizeDownFanoutMt to the PARALLEL_EVAL WNS tail
11. rsz: Add the per-move analysis and the rebuffer accuracy report #11804 rsz: Add the per-move analysis and the rebuffer accuracy report
12. doc: Update the rsz setup repair architecture #11805 doc: Update the rsz setup repair architecture

[github-actions]

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 for batch setup timing repair, replacing several older experimental policies. It adds multi-threaded implementations for key optimization moves (such as RebufferMt, SizeUpMt, SplitLoadMt, SwapPinsMt, and UnbufferMt), integrates a setup repair profiler, and refactors the delay estimator and target collector to support parallel execution. The review feedback highlights critical null pointer dereference risks in DelayEstimator.cc, UnbufferMtGenerator.cc, and SwapPinsMtGenerator.cc when resolving driver pins or top-level ports, and recommends using safer helper methods or adding defensive null checks.

[jhkim-pii]

@codex review

[chatgpt-codex-connector]

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:50.869111Z	c9a3413	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]

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

[maliberty]

The WNS tail removes buffers only in single-move batches, because a removal cannot be reverted in place inside the batch journal.

Why not?

[maliberty]

Claude

== Priority 1 (correctness) ==

• SetupParallelEvalPolicy.cc undoMissedBatch / ParallelEvalState.cc WnsTail::keepSpeculative
  bug: a removed buffer leaves a dangling sta::Instance* behind.
  commitBatch puts target.inst() (the buffer) into batch_instances_. After the
  rollback, that pointer goes into MissTabu (until_, undo_counts_, excluded_) and
  into speculative_instances_.
  Two consequences:
  (a) The restored buffer is a new instance, so it is not on the tabu list. The
  same removal can win again in the next batch. undo_counts_ never reaches
  max_undos_per_instance, because every retry has a new pointer.
  (b) When the allocator reuses the freed address, an unrelated new instance
  (such as a buffer that was just inserted) can be tabu'd, or permanently
  put in excluded_.
  Suggest recording a removal by something that survives the undo (for example
  the input or output net, or the dbInst id if the journal restores it), or
  leaving removals out of the tabu bookkeeping on purpose.

• ParallelEvalState.hh unbufferAllowed()
  question: removals are limited to single-move batches so that they never need
  a partial salvage. But unbufferAllowed() requires greedy_stalled_, and that is
  the same condition that makes maySpeculate() true. So most removals run in
  speculative batches that are kept and rolled back later across several
  batches. Is that multi-batch journal restore known to be safe with a removal
  in the middle? Later batches may edit the merged net. The comment says
  "cannot be salvaged in place", which only covers the salvage path.

• SetupParallelEvalPolicy.cc generatorAllowed
  question: with -sequence ... unbuffer, a removal is allowed in every
  multi-move batch. A missed batch then skips the salvage
  (batch_salvage_skipped_removal), so one removal discards all the other moves
  in that batch. Is that intended? It weakens the sequence path compared with
  the tail path.

• UnbufferMtGenerator.cc passesMoveConflictGuard
  question: this guard drops kSizeUp, which the serial
  hasBlockingBufferRemovalMove still blocks. So a buffer that an earlier batch
  sized up can now be removed. The comment justifies it, but has the sizeup ->
  remove -> rebuffer churn been checked? In the path_group test, 23 buffers are
  removed while 17 are inserted.

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

• UnbufferMtGenerator.cc copies roughly 300 lines of UnbufferGenerator.cc almost
  verbatim (the context struct, validTarget, resolveDriverContext, the path
  context, and the fanout/cap/slack guards). Suggest moving the shared code into
  a helper both use, so the two cannot drift apart. They already differ in the
  conflict guard and in how slack_scene is set.
• estimateRemovalGain repeats computeNewDelaysSlews and bufferDelay, which
  estimatedSlackOK has just computed for the same buffer, so every eligible
  target runs those delay calcs twice on the main thread. Consider returning the
  gain from the slack check.

[maliberty]

Codex

• Correctness: UnbufferMtCandidate.cc:63 marks only output-net pins as reconnected. removeBuffer() can instead preserve the output net and reconnect the input-net pins. In a multi-move batch, the selector can then miss a conflict. The impact should follow the same surviving-net choice as removal.
• QoR: UnbufferMtGenerator.cc:371 uses the max-cap check’s scene for the slack guard. When there is no max-cap limit, that scene is null and estimatedSlackOK() rejects every removal. The active path scene appears to be the appropriate fallback.

[maliberty]

why 10?