Refs/heads/tage size align repeat index fill align

[CJ362ff]

Summary by CodeRabbit

• Refactor

  • Simplified branch predictor configuration by allowing repeated table-size declarations.

• Bug Fixes

  • Fixed index computation so short history values are tiled to fill the required index width before combining with PC bits.

• Tests

  • Added a unit test that verifies index calculation when folded history is shorter than the index width.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 05ba38e4-21a0-43b9-a2b5-2a0a1ee765b8

📥 Commits

Reviewing files that changed from the base of the PR and between abd7008 and d6f7cc9.

📒 Files selected for processing (2)

• src/cpu/pred/btb/btb_tage.cc
• src/cpu/pred/btb/test/btb_tage.test.cc

✅ Files skipped from review due to trivial changes (1)

• src/cpu/pred/btb/test/btb_tage.test.cc

---

📝 Walkthrough

Walkthrough

Changed BTBTAGE tableSizes syntax, updated getTageIndex to repeat short folded-history chunks across the index width, and added a unit test verifying repeated-bit indexing behavior for short folded histories.

Changes

Cohort / File(s)	Summary
Python configuration src/cpu/pred/BranchPredictor.py	Replaced explicit eight-element tableSizes list with programmatic repetition [2048]*8.
Index computation logic src/cpu/pred/btb/btb_tage.cc	Adjusted BTBTAGE::getTageIndex to use histBits = min(histLengths[t], tableIndexBits[t]), extract that chunk from foldedHist, replicate/append it across the full index bit width, remask, then compute index as (pcBits ^ foldedBits) % tableSizes[t].
Unit test src/cpu/pred/btb/test/btb_tage.test.cc	Added BTBTAGETest.IndexRepeatsShortFoldedHistory to assert expected indices when folded history is shorter than the table index width.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Revert "cpu-o3: Refactor numWays to support per-table configuration" #795 — touches BTBTAGE tableSizes representation and getTageIndex implementation.
• Tage1 align #615 — modifies TAGE index computation in btb_tage related to folded history handling.
• cpu-o3: Refactor numWays to support per-table configuration #793 — related changes to getTageIndex and BTBTAGE configuration.

Suggested Labels

perf, align-kmhv3

Suggested Reviewers

• jensen-yan

Poem

🐰 In a burrow of bits I softly tread,
Short histories hop, then loop overhead.
I spin the small chunk, repeat it anew,
Indices shimmer — a pattern in view. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is vague and uses non-descriptive terminology that obscures the actual changes; it reads like a git branch name rather than a clear summary of the modification.	Replace with a clear, descriptive title such as 'Fix BTBTAGE folded history index calculation for short histories' or 'Refactor BTBTAGE index computation to handle histories shorter than index width'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch tage-size-align-repeat-index-fill-align

---

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

[XiangShanRobot]

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

Align BTB Performance

Overall Score

	PR	Master	Diff(%)
Score	18.49	18.63	-0.74 🔴

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

Align BTB Performance

Overall Score

	PR	Previous Commit	Diff(%)
Score	18.49	18.27	+1.20 🟢

[coderabbitai]

🧹 Nitpick comments (1)

src/cpu/pred/btb/test/btb_tage.test.cc (1)

409-426: Pin the test to an explicit predictor shape.

These expectations only hold while the UNIT_TEST constructor keeps 10 index bits and {4, 8, ...} history lengths. If those defaults move, this test will fail without any regression in the repetition logic. Instantiate a local BTBTAGE with explicit constructor arguments here.

♻️ Suggested tightening

 TEST_F(BTBTAGETest, IndexRepeatsShortFoldedHistory) {
+    BTBTAGE local_tage(/*numPredictors=*/2, /*numWaysPerTable=*/1,
+                       /*tableSize=*/1024, /*numBanks=*/1);
     const Addr pc = 0x0;
@@
-    EXPECT_EQ(tage->getTageIndex(pc, t0, t0FoldedHist), t0ExpectedFolded)
+    EXPECT_EQ(local_tage.getTageIndex(pc, t0, t0FoldedHist), t0ExpectedFolded)
         << "t0 index should repeat 4-bit history to cover all index bits";
@@
-    EXPECT_EQ(tage->getTageIndex(pc, t1, t1FoldedHist), t1ExpectedFolded)
+    EXPECT_EQ(local_tage.getTageIndex(pc, t1, t1FoldedHist), t1ExpectedFolded)
         << "t1 index should repeat 8-bit history to cover all index bits";
 }

🤖 Prompt for AI Agents

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

In `@src/cpu/pred/btb/test/btb_tage.test.cc` around lines 409 - 426, The test
relies on implicit UNIT_TEST defaults (10 index bits and history lengths
{4,8,...}); to make it robust, create a local BTBTAGE instance in the test with
explicit constructor arguments that pin indexBits=10 and the matching per-table
history lengths (e.g., 4 and 8 for tables 0 and 1), then call that instance's
getTageIndex instead of the shared/implicit member (replace usages of
`tage->getTageIndex` with the local `BTBTAGE` instance). Ensure the local
constructor arguments exactly match the expected folded values used in the
assertions so the test no longer depends on UNIT_TEST defaults.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/cpu/pred/btb/test/btb_tage.test.cc`:
- Around line 409-426: The test relies on implicit UNIT_TEST defaults (10 index
bits and history lengths {4,8,...}); to make it robust, create a local BTBTAGE
instance in the test with explicit constructor arguments that pin indexBits=10
and the matching per-table history lengths (e.g., 4 and 8 for tables 0 and 1),
then call that instance's getTageIndex instead of the shared/implicit member
(replace usages of `tage->getTageIndex` with the local `BTBTAGE` instance).
Ensure the local constructor arguments exactly match the expected folded values
used in the assertions so the test no longer depends on UNIT_TEST defaults.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18045b4f-4051-481e-9a02-5d2cd4986224

📥 Commits

Reviewing files that changed from the base of the PR and between 54de78c and 99ca8e0.

📒 Files selected for processing (3)

• src/cpu/pred/BranchPredictor.py
• src/cpu/pred/btb/btb_tage.cc
• src/cpu/pred/btb/test/btb_tage.test.cc

[github-actions]

🚀 Coremark Smoke Test Results

Branch	IPC	Change
Base (xs-dev)	2.2605	-
This PR	2.2567	📉 -0.0037 (-0.17%)

✅ Difftest smoke test passed!

[github-actions]

🚀 Coremark Smoke Test Results

Branch	IPC	Change
Base (xs-dev)	2.2605	-
This PR	2.2637	📈 +0.0032 (+0.14%)

✅ Difftest smoke test passed!

[XiangShanRobot]

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

Align BTB Performance

Overall Score

	PR	Master	Diff(%)
Score	18.47	18.63	-0.82 🔴

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

Align BTB Performance

Overall Score

	PR	Previous Commit	Diff(%)
Score	18.47	18.49	-0.09 🔴

[github-actions]

🚀 Coremark Smoke Test Results

Branch	IPC	Change
Base (xs-dev)	2.2605	-
This PR	2.2567	📉 -0.0037 (-0.17%)

✅ Difftest smoke test passed!

[XiangShanRobot]

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

Align BTB Performance

Overall Score

	PR	Master	Diff(%)
Score	18.49	18.73	-1.31 🔴

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

Align BTB Performance

Overall Score

	PR	Previous Commit	Diff(%)
Score	18.49	18.47	+0.09 🟢