Microtage merage

[CJ362ff]

Summary by CodeRabbit

Release Notes

New Features

• Added MicroTAGE branch predictor with expanded configurability including table sizes, history depths, and banking conflict handling for improved branch prediction accuracy.
• MicroTAGE is now enabled in supported CPU configurations with optimized default parameters.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces a new MicroTAGE branch predictor class that replaces a simpler BTBTAGE-based design with an expanded, bank-aware, multi-table TAGE predictor. Changes include configuration updates, build system integration, type signature modifications, and a comprehensive 1,100+ line implementation with prediction, update, and history management logic.

Changes

Cohort / File(s)	Summary
Configuration Updates configs/example/idealkmhv3.py, configs/example/kmhv3.py	Added comment documenting microtage configuration; enabled MicroTAGE in DecoupledBPUWithBTB (changed from False to True).
Build System Integration src/cpu/pred/SConscript	Added MicroTAGE to SimObject BP type list, included new source file btb/microtage.cc, and introduced new DebugFlag UTAGE.
Predictor Type Definitions src/cpu/pred/BranchPredictor.py	Refactored MicroTAGE from BTBTAGE subclass to TimedBaseBTBPredictor with extensive new public parameters (needMoreHistories, updateOnRead, numPredictors, tableSizes, TTagBitSizes, TTagPcShifts, blockSize, histLengths, banking configs, etc.); updated DecoupledBPUWithBTB.microtage field type from Param.BTBTAGE to Param.MicroTAGE.
Interface Updates src/cpu/pred/btb/decoupled_bpred.hh	Added microtage.hh include; changed private member type from BTBTAGE\* to MicroTAGE\*.
MicroTAGE Implementation src/cpu/pred/btb/microtage.hh, src/cpu/pred/btb/microtage.cc	Introduced complete MicroTAGE predictor with bank-aware multi-table TAGE design, including TageEntry/TageTableInfo/TagePrediction/TageMeta nested structures, core prediction/update workflows (generateSinglePrediction, lookupHelper, putPCHistory, update, canResolveUpdate, doResolveUpdate), folded history management (doUpdateHist, specUpdatePHist, recoverPHist), banking logic (getBankId, enableBankConflict), and extensive TageStats instrumentation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• PR #644: Adds the two-phase resolveUpdate API (canResolveUpdate/doResolveUpdate) and blocking/notify flow for BTB/TAGE updates that directly align with MicroTAGE's update resolution infrastructure.
• PR #628: Modifies branch predictors to adopt path-folded history interfaces (specUpdatePHist/recoverPHist, doUpdateHist) that are now central to MicroTAGE's history management.
• PR #615: Extends TAGE-style BTB predictor with bank-aware, position-aware designs (bank params, getTageTag/getTageIndex/getBankId) that mirror MicroTAGE's banking model.

Suggested labels

align-kmhv3

Suggested reviewers

• Yakkhini

Poem

🐰 A tiny TAGE hops into view,
With folded histories, tables, and banks so true,
Banking on conflicts, it predicts with grace,
Microtagely swift through the fetch-decode race! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'Microtage merage' is unclear and contains a typo; it does not clearly describe the main changes.	Replace with a clear, specific title such as 'Implement MicroTAGE branch predictor with TAGE-based BTB integration' to accurately reflect the substantial additions of new predictor classes and configuration parameters.
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.35% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch microtage-ok-type

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (7)

src/cpu/pred/btb/microtage.hh (1)

163-165: Consider documenting the UNIT_TEST visibility change.

The #ifndef UNIT_TEST guard around private: exposes internal methods during testing, which is a reasonable pattern. A brief comment explaining this design choice would help future maintainers understand the intent.

📝 Suggested documentation

+// In production builds, the following methods are private implementation details.
+// In UNIT_TEST builds, they are exposed for direct testing.
 `#ifndef` UNIT_TEST
   private:
 `#endif`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.hh` around lines 163 - 165, Add a brief comment
above the `#ifndef` UNIT_TEST guard in microtage.hh explaining that the
conditional exposes internal members for unit testing; mention that the guard
toggles visibility of the subsequent private: section so test code can access
internal methods/state while production builds keep them private (reference the
UNIT_TEST macro and the private: label). Keep the comment concise and near the
guard so future maintainers understand the intent.

configs/example/idealkmhv3.py (1)

83-83: Clarify the intent of this commented-out configuration.

This commented-out line suggests MicroTAGE should be disabled in the "ideal" configuration, but since it's commented out, MicroTAGE will use its default enabled state (True per BranchPredictor.py).

If the intent is to keep MicroTAGE disabled in this ideal configuration (for performance comparison or baseline testing), consider uncommenting it. Otherwise, if MicroTAGE should be enabled here too, consider removing this commented line to avoid confusion.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@configs/example/idealkmhv3.py` at line 83, The commented config line
"cpu.branchPred.microtage.enabled = False" leaves MicroTAGE using its default
True value; decide and apply the intended behavior: if MicroTAGE should be
disabled in this ideal config, uncomment "cpu.branchPred.microtage.enabled =
False" (so the setting is explicit), otherwise remove the commented line to
avoid confusion and rely on the default True from BranchPredictor.py; update the
file accordingly so the intent is explicit.

src/cpu/pred/btb/microtage.cc (5)

772-783: Add defensive bounds check for index bit width.

Similar to the tag computation, if tableIndexBits[t] reaches 64 or more, the shift would be undefined behavior. Consider adding an assertion.

🛡️ Suggested defensive fix

 Addr
 MicroTAGE::getTageIndex(Addr pc, int t, uint64_t foldedHist)
 {
     // Create mask for tableIndexBits[t] to limit result size
+    assert(tableIndexBits[t] < 64 && "Index bit width must be less than 64");
     Addr mask = (1ULL << tableIndexBits[t]) - 1;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.cc` around lines 772 - 783, In getTageIndex you
must guard against undefined behavior when tableIndexBits[t] is 64 or more
before doing (1ULL << tableIndexBits[t]); add a defensive bounds check (e.g.,
assert or conditional) that tableIndexBits[t] is in the valid range (1..63) or
handle the 64-bit case explicitly by using a full-mask (e.g., mask = ~0ULL) —
update the code around tableIndexBits[t], mask, and pcShift in
MicroTAGE::getTageIndex to mirror the safe check used in the tag computation so
shifts are never undefined.

---

265-273: Consider early-exit optimization for empty btbEntries.

If btbEntries is empty, the loop does nothing. An early return could slightly improve readability, though this is minor.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.cc` around lines 265 - 273, The loop over
btbEntries in microtage.cc performs no work when btbEntries is empty; add an
early exit by checking btbEntries.empty() before the loop (e.g., at the start of
the function or just before the for) and return immediately to improve
readability; locate the block that calls generateSinglePrediction(btb_entry,
startPC), writes to meta->preds[btb_entry.pc], calls
tageStats.updateStatsWithTagePrediction(pred, true), and pushes into results and
insert the empty-check there.

---

91-97: Potential redundant resize of tableTagBits.

tableTagBits is resized at line 93, but it was already initialized from p.TTagBitSizes at line 68 (or sized in the test constructor at line 55). The resize at line 93 appears unnecessary and could silently mask configuration issues where TTagBitSizes has fewer elements than numPredictors.

Consider either removing the resize or adding a more explicit validation error:

♻️ Suggested clarification

     tageTable.resize(numPredictors);
     tableIndexBits.resize(numPredictors);
-    tableTagBits.resize(numPredictors);
+    // tableTagBits already initialized from params; validate size matches
+    assert(tableTagBits.size() >= numPredictors && "TTagBitSizes must have numPredictors elements");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.cc` around lines 91 - 97, The resize of
tableTagBits is redundant and can mask misconfiguration of p.TTagBitSizes;
instead of silently resizing, validate that p.TTagBitSizes contains entries for
all predictors (compare p.TTagBitSizes.size() to numPredictors) and if it
doesn't, either assert/throw a clear configuration error or fallback explicitly
with logged intent; remove the tableTagBits.resize(numPredictors) call and
replace it with a check that sets tableTagBits from p.TTagBitSizes (or fails
fast), leaving the existing tablePcShifts resize behavior as-is.

---

628-632: Graceful handling when prediction metadata is missing.

The code logs and returns when predMeta is null, which is good defensive programming. However, this could indicate a bug in the calling code if it happens unexpectedly.

Consider adding a stat counter to track how often this occurs, which would help diagnose issues in production:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.cc` around lines 628 - 632, Add a stat counter
increment when prediction metadata is missing: in the block where predMeta is
obtained (auto predMeta =
std::static_pointer_cast<TageMeta>(stream.predMetas[getComponentIdx()])), before
returning on the null check, increment a descriptive counter (e.g.,
predMetaMissingCount or nullPredMetaStat) defined on the containing class/module
(e.g., Microtage or associated stats struct) so occurrences are tracked in
production; keep the existing DPRINTF(UTAGE, ...) and return behavior unchanged
but ensure the new counter is declared/registered with the file's stat framework
and incremented each time predMeta is null.

---

756-770: Add defensive bounds check for tag bit width.

The static analyzer flagged a potential undefined behavior: if tableTagBits[t] is 64 or greater, (1ULL << tableTagBits[t]) is undefined behavior. While current defaults (16 bits) are safe, a misconfigured parameter could trigger this.

🛡️ Suggested defensive fix

 Addr
 MicroTAGE::getTageTag(Addr pc, int t, uint64_t foldedHist, uint64_t altFoldedHist, Addr position)
 {
     // Create mask for tableTagBits[t] to limit result size
+    assert(tableTagBits[t] < 64 && "Tag bit width must be less than 64");
     Addr mask = (1ULL << tableTagBits[t]) - 1;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cpu/pred/btb/microtage.cc` around lines 756 - 770, Add a defensive bounds
check before computing mask from tableTagBits[t]: avoid doing (1ULL <<
tableTagBits[t]) when tableTagBits[t] >= 64 by computing mask conditionally
(e.g., if tableTagBits[t] >= 64 set mask = ~0ULL; else mask = (1ULL <<
tableTagBits[t]) - 1). Update the block that currently computes Addr mask =
(1ULL << tableTagBits[t]) - 1; (in the microtage tag/XOR construction using
tableTagBits[t], pcBits, foldedHist, altFoldedHist, bankBaseShift, position) to
use this guarded computation so shifts cannot be invoked with >=64.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@configs/example/idealkmhv3.py`:
- Line 83: The commented config line "cpu.branchPred.microtage.enabled = False"
leaves MicroTAGE using its default True value; decide and apply the intended
behavior: if MicroTAGE should be disabled in this ideal config, uncomment
"cpu.branchPred.microtage.enabled = False" (so the setting is explicit),
otherwise remove the commented line to avoid confusion and rely on the default
True from BranchPredictor.py; update the file accordingly so the intent is
explicit.

In `@src/cpu/pred/btb/microtage.cc`:
- Around line 772-783: In getTageIndex you must guard against undefined behavior
when tableIndexBits[t] is 64 or more before doing (1ULL << tableIndexBits[t]);
add a defensive bounds check (e.g., assert or conditional) that
tableIndexBits[t] is in the valid range (1..63) or handle the 64-bit case
explicitly by using a full-mask (e.g., mask = ~0ULL) — update the code around
tableIndexBits[t], mask, and pcShift in MicroTAGE::getTageIndex to mirror the
safe check used in the tag computation so shifts are never undefined.
- Around line 265-273: The loop over btbEntries in microtage.cc performs no work
when btbEntries is empty; add an early exit by checking btbEntries.empty()
before the loop (e.g., at the start of the function or just before the for) and
return immediately to improve readability; locate the block that calls
generateSinglePrediction(btb_entry, startPC), writes to
meta->preds[btb_entry.pc], calls tageStats.updateStatsWithTagePrediction(pred,
true), and pushes into results and insert the empty-check there.
- Around line 91-97: The resize of tableTagBits is redundant and can mask
misconfiguration of p.TTagBitSizes; instead of silently resizing, validate that
p.TTagBitSizes contains entries for all predictors (compare
p.TTagBitSizes.size() to numPredictors) and if it doesn't, either assert/throw a
clear configuration error or fallback explicitly with logged intent; remove the
tableTagBits.resize(numPredictors) call and replace it with a check that sets
tableTagBits from p.TTagBitSizes (or fails fast), leaving the existing
tablePcShifts resize behavior as-is.
- Around line 628-632: Add a stat counter increment when prediction metadata is
missing: in the block where predMeta is obtained (auto predMeta =
std::static_pointer_cast<TageMeta>(stream.predMetas[getComponentIdx()])), before
returning on the null check, increment a descriptive counter (e.g.,
predMetaMissingCount or nullPredMetaStat) defined on the containing class/module
(e.g., Microtage or associated stats struct) so occurrences are tracked in
production; keep the existing DPRINTF(UTAGE, ...) and return behavior unchanged
but ensure the new counter is declared/registered with the file's stat framework
and incremented each time predMeta is null.
- Around line 756-770: Add a defensive bounds check before computing mask from
tableTagBits[t]: avoid doing (1ULL << tableTagBits[t]) when tableTagBits[t] >=
64 by computing mask conditionally (e.g., if tableTagBits[t] >= 64 set mask =
~0ULL; else mask = (1ULL << tableTagBits[t]) - 1). Update the block that
currently computes Addr mask = (1ULL << tableTagBits[t]) - 1; (in the microtage
tag/XOR construction using tableTagBits[t], pcBits, foldedHist, altFoldedHist,
bankBaseShift, position) to use this guarded computation so shifts cannot be
invoked with >=64.

In `@src/cpu/pred/btb/microtage.hh`:
- Around line 163-165: Add a brief comment above the `#ifndef` UNIT_TEST guard in
microtage.hh explaining that the conditional exposes internal members for unit
testing; mention that the guard toggles visibility of the subsequent private:
section so test code can access internal methods/state while production builds
keep them private (reference the UNIT_TEST macro and the private: label). Keep
the comment concise and near the guard so future maintainers understand the
intent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b05f35f2-1ad6-4647-9523-5ff4bd41cc64

📥 Commits

Reviewing files that changed from the base of the PR and between 318c57c and 9e6be62.

📒 Files selected for processing (7)

• configs/example/idealkmhv3.py
• configs/example/kmhv3.py
• src/cpu/pred/BranchPredictor.py
• src/cpu/pred/SConscript
• src/cpu/pred/btb/decoupled_bpred.hh
• src/cpu/pred/btb/microtage.cc
• src/cpu/pred/btb/microtage.hh

[github-actions]

🚀 Coremark Smoke Test Results

Branch	IPC	Change
Base (xs-dev)	2.2203	-
This PR	2.2219	📈 +0.0016 (+0.07%)

✅ Difftest smoke test passed!

[XiangShanRobot]

[Generated by GEM5 Performance Robot]
commit: 9e6be62
workflow: gem5 Align BTB Performance Test(0.3c)

Align BTB Performance

Overall Score

	PR	Master	Diff(%)
Score	17.91	17.84	+0.36 🟢