[codex] Add standalone SC switch#905
Conversation
Change-Id: I782cb5acc1c54ef4018cdec025bd9e954b1b3222
|
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 (2)
📝 WalkthroughWalkthroughThis PR enables standalone SC (MGSC) mode by adding a new ChangesStandalone SC Mode with TAGE Fallback
Sequence DiagramsequenceDiagram
participant CLI as Xiangshan CLI
participant KMH as KMHv3 setup
participant BTB as BTBMGSC
participant TAGE as TageProvider
CLI->>KMH: set --standalone-sc
KMH->>BTB: set allowMissingTageInfo / disable TAGE sources
BTB->>TAGE: request TageInfoForMGSC (PC,startPC,tid,asid)
TAGE-->>BTB: return TageInfo or missing
alt TageInfo missing and allowMissingTageInfo == false
BTB->>BTB: panic_if(missing TageInfo)
else TageInfo missing but allowed
BTB->>BTB: use default TageInfoForMGSC and continue prediction
end
BTB->>BTB: generate prediction, push trace (useSc, scPred, scWrong)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in “standalone SC” mode for XiangShan’s kmhv3 configuration, allowing experiments to disable MicroTAGE/TAGE while still running MGSC by using a defined default TAGE-metadata fallback path.
Changes:
- Add
--standalone-scCLI option and wire it intokmhv3.pyto disable MicroTAGE/TAGE and force MGSC to use SC output. - Extend MGSC with
allowMissingTageInfoto permit a deliberate “missing TAGE metadata” fallback (while keeping the default path strict viapanic_if). - Add a focused MGSC unit test covering the missing-TAGE fallback behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cpu/pred/btb/test/btb_mgsc.test.cc | Adds a unit test ensuring MGSC behaves correctly when TAGE metadata is intentionally absent. |
| src/cpu/pred/btb/btb_mgsc.hh | Adds the allowMissingTageInfo flag and exposes it via TestAccess. |
| src/cpu/pred/btb/btb_mgsc.cc | Replaces assert(false) with a panic_if + explicit default fallback when allowed. |
| src/cpu/pred/BranchPredictor.py | Adds the SimObject param for allowMissingTageInfo so it can be configured from Python configs. |
| configs/example/kmhv3.py | Implements --standalone-sc behavior by disabling MicroTAGE/TAGE and forcing MGSC SC-only mode. |
| configs/common/xiangshan.py | Adds the --standalone-sc argument to the XiangShan config parser. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 1d42b8ce0d
ℹ️ 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".
Change-Id: Id21ffa64f328c7f6dc9903004d10baa16c900fa9
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
Change-Id: Idd6fd6a6a7ab6a272ecba38de85bcfba6735f5ad
Change-Id: I1c71019b49e59f75d6bf701e426bd868a4cd8066
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
1 similar comment
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
Motivation
Add a small opt-in switch for standalone SC experiments so TAGE can be disabled from the mainline configuration without maintaining a long-lived experiment branch.
Approach
--standalone-scto the XiangShan config options.kmhv3.py, disable MicroTAGE and TAGE for this mode, then force MGSC to use SC output.allowMissingTageInfoso MGSC can explicitly run with default TAGE metadata only when TAGE is intentionally disabled.panic_ifwhen MGSC is missing TAGE metadata unexpectedly.This PR intentionally does not include IMHist or skill/documentation updates.
Validation
git diff --checkscons build/RISCV/cpu/pred/btb/test/mgsc.test.debug --unit-test -j64./build/RISCV/cpu/pred/btb/test/mgsc.test.debugscons build/RISCV/gem5.opt --gold-linker -j16timeout 20s ./build/RISCV/gem5.opt configs/example/kmhv3.py --help | rg -- '--standalone-sc'Note: an initial
gem5.opt -j64build hit a generated-header parallel dependency race, then the-j16rebuild completed successfully.Summary by CodeRabbit
New Features
--standalone-sccommand-line flag to enable standalone SC prediction mode (disables direction TAGE sources).Tests
Chores