Repository navigation
Conversation
|
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[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the experimental PARALLEL_EVAL setup repair policy (SetupParallelEvalPolicy), which optimizes setup timing in parallel batches using worker threads. It removes older experimental policies (LEGACY_MT, MT1, MEASURED_VT_SWAP) and adds several supporting components, including a bottleneck analysis tool, a batch selector, a repair setup profiler, and multi-threaded move generators (RebufferMtGenerator, SplitLoadMtGenerator, SwapPinsMtGenerator). The review feedback is highly constructive, pointing out several critical locations where null pointer dereferences could occur, specifically regarding candidate_scene_cell in DelayEstimator.cc, input_ in both SplitLoadMtCandidate.cc and SwapPinsMtCandidate.cc, prepared_timing_ in SequentialTimingEstimator.cc, stage.arc.scene/min_max in SetupParallelEvalPolicy.cc, and the ports in OptimizerTypes.cc.
1adb2bd to
7448902
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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>
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>
7448902 to
e9ea89d
Compare
2 participants
Part 8 of 12 of the PARALLEL_EVAL setup repair stack. It builds on #11800 and also shows the commits of parts 1 to 7; its own change is the last commit.
Summary
Changes
Resizer::swapPinDelays()is split out offindSwapPinCandidate(), so both forms compare the same arc delays.SwapPinsMtGeneratormeasures the arc delay through each equivalent input of the target gate on the main thread and picks the fastest one on a worker; the displaced signal pays the same delay difference in the score.-sequenceacceptsswap; without-sequencethe WNS tail adds SwapPinsMt, and-skip_pin_swapkeeps it out.flowchart LR P["critical signal on input A"] --> G["equivalent inputs A, B<br/>(arc delays measured on the main thread)"] G --> C{"B faster than A?"} C -- yes --> S["swap A and B;<br/>the displaced signal pays the difference"] C -- no --> N["no candidate"]Stack