cpu-o3: Avoid SMT frontend work during pending redirects#935
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a ChangesRedirect-Pending Signal Propagation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jensen-yan
force-pushed
the
codex/smt-bpu-redirect-switch
branch
from
54cf080 to
09724f8
Compare
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
jensen-yan
force-pushed
the
codex/smt-bpu-redirect-switch
branch
from
09724f8 to
9665ee5
Compare
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
jensen-yan
force-pushed
the
codex/smt-bpu-redirect-switch
branch
from
9665ee5 to
de6987d
Compare
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
There was a problem hiding this comment.
Pull request overview
This PR adds an IEW→Fetch “redirect pending” hint to improve SMT frontend efficiency during the window between backend redirect detection and the delayed formal squash reaching Fetch, and also fixes per-thread resolvedCFIs cleanup in IEW.
Changes:
- Introduce
TimeStruct::IewComm::redirectPendingand propagate it from IEW to Fetch/BPU to steer SMT scheduling away from redirecting threads. - Extend FTQ target selection to accept an eligibility mask and collect “ineligible skip” counters for visibility.
- Fix IEW’s SMT resolved-CFI cleanup to clear per-thread
resolvedCFIsfor all threads each cycle and add new stats for redirect-pending effects.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cpu/pred/btb/ftq.hh | Add eligibility-aware getTargetTid overload for FTQ selection. |
| src/cpu/pred/btb/ftq.cc | Implement eligibility mask + ineligible skip counting in FTQ round-robin selection. |
| src/cpu/pred/btb/decoupled_bpred.hh | Add per-thread redirectPending state, scheduling helpers, and stats declarations. |
| src/cpu/pred/btb/decoupled_bpred.cc | Enforce redirect-pending eligibility in scheduler; clear redirect-pending on squash/reset; add stats increments. |
| src/cpu/pred/btb/decoupled_bpred_stats.cc | Register new DBP/BTB scheduler and redirect-pending statistics. |
| src/cpu/o3/iew.cc | Drive redirectPending on redirect acceptance; clear per-thread resolvedCFIs/redirectPending each cycle. |
| src/cpu/o3/fetch.hh | Track redirect-pending state/counters in Fetch; add Fetch stats for redirect-pending skips. |
| src/cpu/o3/fetch.cc | Implement redirect-pending lifecycle and eligibility-aware FTQ head selection in Fetch. |
| src/cpu/o3/comm.hh | Add redirectPending field to IEW→Fetch per-cycle comm struct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (redirectPendingCycles[tid] == 0) { | ||
| redirectPending[tid] = false; | ||
| dbpbtb->setRedirectPending(tid, false); | ||
| } else { | ||
| --redirectPendingCycles[tid]; | ||
| } |
| ThreadID curTid = scheduleThread(); | ||
| if (curTid == InvalidThreadID) { | ||
| bool anyActiveThread = false; | ||
| for (ThreadID tid = 0; tid < numThreads; ++tid) { | ||
| if (isThreadActive(tid)) { | ||
| anyActiveThread = true; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de6987d033
ℹ️ 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".
| void | ||
| DecoupledBPUWithBTB::setRedirectPending(ThreadID tid, bool pending) | ||
| { | ||
| threads[tid].redirectPending = pending; |
There was a problem hiding this comment.
When the IEW hint arrives after this thread has already produced a validprediction (for example while waiting out override bubbles or shared FTQ space), this setter only flips redirectPending. tick() still calls processNewPrediction(tid) for every thread, and that path does not check redirectPending, so the redirecting thread can still enqueue a wrong-path FTQ entry and update speculative history during the pending window. That consumes the same BPU/FTQ resources this change is trying to avoid until the formal squash arrives; consider clearing validprediction/override bubbles or blocking processNewPrediction when pending is true.
Useful? React with 👍 / 👎.
Change-Id: I14dd1e33b5538ae3e80a23456725230e2c207ba3
jensen-yan
force-pushed
the
codex/smt-bpu-redirect-switch
branch
from
de6987d to
a0d15eb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d15eb4e3
ℹ️ 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".
| if (numThreads > 1 && fromIEW->iewInfo[tid].redirectPending) { | ||
| redirectPending[tid] = true; | ||
| redirectPendingCycles[tid] = redirectPendingHoldCycles; | ||
| dbpbtb->setRedirectPending(tid, true); |
There was a problem hiding this comment.
In SMT configs where the IEW hint is not earlier than the formal commit squash, such as configs/example/kmhv3.py when run with --smt (iewToFetchDelay == commitToFetchDelay == 2), initializeTickState() handles fromCommit first and clears the pending state, then calls handleIEWSignals() and this block immediately sets it again from the same delayed redirect. The recovered thread is then made ineligible for BPU/FTQ selection for the guard window even though the squash has already been applied, adding redirect bubbles after every such recovery; gate this hint on an actual IEW-before-commit delay or suppress it for a tid that just consumed a formal squash.
Useful? React with 👍 / 👎.
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
|
Motivation
SMT frontend performance is sensitive to long redirect / flush latency. After IEW detects a redirect, Fetch and the decoupled BPU can still spend work on the redirecting thread until the formal delayed squash reaches Fetch. This PR adds a narrow redirect-pending contract so the frontend can prefer other eligible SMT threads during that window.
It also fixes the SMT resolved-CFI cleanup path: IEW now clears per-thread
resolvedCFIsfor all threads before producing new per-cycle entries, instead of only clearing*activeThreads->begin().Approach
TimeStruct::IewComm::redirectPendingas an IEW-to-Fetch per-cycle signal.Scope and modeling notes
This does not change formal squash timing, PC recovery, or history recovery. The new signal models the performance consequence of backend redirect detection: avoid continuing to supply known wrong-path frontend work while waiting for the existing delayed squash path.
The redirect-pending state is deliberately not a functional latch. It is only enabled when
numThreads > 1, and it has a bounded lifetime derived from the gap betweeniewToFetchDelayandcommitToFetchDelay. This keeps the optimization scoped to SMT thread selection and avoids starving single-thread/non-SMT runs if a speculative early hint is not followed by a formal squash in the expected window.Fetch still handles commit/decode squash before consuming IEW resolve-update entries. This ordering matters: putting resolve-update enqueue before the formal squash path can train/enqueue stale resolved CFIs and severely hurt non-SMT coremark IPC.
Hot-path complexity remains bounded by
numThreads.Validation
scons build/RISCV/gem5.opt --gold-linker -j64GCBV_REF_SO=/nfs/home/share/gem5_ci/ref/normal/riscv64-nemu-notama-tvalref-so ./build/RISCV/gem5.opt -d /tmp/pr935-orderfix-nonsmt ./configs/example/kmhv3.py --raw-cpt --generic-rv-cpt=/nfs/home/share/gem5_ci/checkpoints/coremark-riscv64-xs.binm5_exitat tick453097116system.cpu.ipc = 2.316167redirectPendingPredictionSkips = 0, confirming the early redirect gate is inactive in non-SMT mode./build/RISCV/gem5.opt -d /tmp/pr935-orderfix-smt ./configs/example/smt_idealkmhv3.py --raw-cpt --generic-rv-cpt=/nfs/home/share/gem5_ci/checkpoints/coremark-riscv64-xs.bin --difftest-ref-so=/nfs/home/share/gem5_ci/ref/multi/riscv64-nemu-interpreter-so --maxinsts=0m5_exitat tick604296765ipc::0/1 ~= 1.73664redirectPendingPredictionSkips = 33376redirectPendingFetchSkips = 23932redirectPendingOnlyFetchCycles = 2240Local SMT coremark comparison against
xs-devon the same checkpoint:xs-dev:simTicks = 613750968,system.cpu.numCycles = 1843097,ipc::0/1 ~= 1.70989simTicks = 604296765,system.cpu.numCycles = 1814706,ipc::0/1 ~= 1.73664This is only a small local smoke/perf signal, not a replacement for CI or SPEC SMT performance runs.
Summary by CodeRabbit
New Features
Bug Fixes