Skip to content

cpu-o3: Update numWays references to support per-table#797

Closed
CJ362ff wants to merge 2 commits intoxs-devfrom
tage1-table-tuning-align
Closed

cpu-o3: Update numWays references to support per-table#797
CJ362ff wants to merge 2 commits intoxs-devfrom
tage1-table-tuning-align

Conversation

@CJ362ff
Copy link
Copy Markdown
Contributor

@CJ362ff CJ362ff commented Mar 17, 2026

Change-Id: Ieda3140c914857d771b53248b3670a2d97cef87c

Summary by CodeRabbit

  • Refactor

    • Optimized branch predictor table capacity calculations for improved efficiency
    • Streamlined internal parameter structure declarations
  • Tests

    • Updated branch predictor test suite to reflect internal implementation changes

Change-Id: Ieda3140c914857d771b53248b3670a2d97cef87c
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

This PR contains formatting and logic refinements to the BTBTAGE branch predictor. Parameter declarations are reformatted to single-line format in BranchPredictor.py, table capacity computation is updated to use indexed array access in btb_tage_ub.cc, and test utilities are updated to use direct array indexing instead of accessor functions.

Changes

Cohort / File(s) Summary
Parameter Declaration Formatting
src/cpu/pred/BranchPredictor.py
Three public VectorParam.Unsigned parameters (tableSizes, histLengths, numWays) reformatted from multiline to single-line declarations. Default values and types remain unchanged.
Predictor Logic Updates
src/cpu/pred/btb/btb_tage_ub.cc
Updated table capacity calculation in initUpperBoundState to use indexed numWays[i] instead of scalar access; enhanced TagePrediction constructor calls to include finalProviderTable and finalProviderIsAlt parameters.
Test Utility Refactoring
src/cpu/pred/btb/test/btb_tage.test.cc
Replaced getNumWays(t) accessor calls with direct array access to numWays[t] across multiple test functions and loop bounds.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Possibly related PRs

Suggested labels

align-kmhv3

Suggested reviewers

  • jensen-yan

Poem

🐰 With whiskers twitched, this rabbit hops with glee,
Array indices bounce—no more getNumWays(), you see!
Parameters line up, formatting so neat,
Predictions flow faster, the refactor complete! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: updating numWays references to support per-table configuration, which is reflected in the code changes across the three modified files.

✏️ 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
  • Commit unit tests in branch tage1-table-tuning-align
📝 Coding Plan
  • Generate coding plan for human review comments

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

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

Tip

You can customize the high-level summary generated by CodeRabbit.

Configure the reviews.high_level_summary_instructions setting to provide custom instructions for generating the high-level summary.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

235-235: Keep getNumWays() (or add explicit bounds asserts) for loop bounds in tests.

Direct tage->numWays[t] indexing drops the table-index bounds assertion provided by getNumWays(t), so a misconfigured numWays vector can become undefined access instead of a clear test failure. Please either keep getNumWays(t) here or add ASSERT_LT(t, tage->numWays.size()) before indexing.

Also applies to: 264-264, 800-800, 816-816, 828-828, 834-834, 838-838, 858-858

🤖 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` at line 235, The test currently
indexes tage->numWays[t] directly which bypasses the bounds check provided by
getNumWays(t); change the loop to use tage->getNumWays(t) for bounds (or, if you
prefer direct access, add an assertion like ASSERT_LT(t, tage->numWays.size())
before any indexing) so misconfigured numWays produces a clear test failure;
update the occurrences around the loops that reference numWays (e.g., where you
currently have for (unsigned way = 0; way < tage->numWays[t]; way++)) to use
getNumWays(t) or add the explicit ASSERT_LT check.
🤖 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`:
- Line 235: The test currently indexes tage->numWays[t] directly which bypasses
the bounds check provided by getNumWays(t); change the loop to use
tage->getNumWays(t) for bounds (or, if you prefer direct access, add an
assertion like ASSERT_LT(t, tage->numWays.size()) before any indexing) so
misconfigured numWays produces a clear test failure; update the occurrences
around the loops that reference numWays (e.g., where you currently have for
(unsigned way = 0; way < tage->numWays[t]; way++)) to use getNumWays(t) or add
the explicit ASSERT_LT check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 66a36b4c-3a01-42a3-b5e5-50dcd5f684d7

📥 Commits

Reviewing files that changed from the base of the PR and between 6b87b99 and 1d6095b.

📒 Files selected for processing (3)
  • src/cpu/pred/BranchPredictor.py
  • src/cpu/pred/btb/btb_tage_ub.cc
  • src/cpu/pred/btb/test/btb_tage.test.cc

@CJ362ff CJ362ff closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant