Skip to content

Tage trace fix#822

Merged
CJ362ff merged 3 commits intoxs-devfrom
tage-trace-fix
Apr 10, 2026
Merged

Tage trace fix#822
CJ362ff merged 3 commits intoxs-devfrom
tage-trace-fix

Conversation

@CJ362ff
Copy link
Copy Markdown
Contributor

@CJ362ff CJ362ff commented Apr 10, 2026

Summary by CodeRabbit

  • Changes
    • Modified branch prediction update behavior by disabling "update on read" functionality by default
    • Corrected prediction trace recording to capture original prediction data instead of recomputed values for improved accuracy

Cao Jiaming added 3 commits April 10, 2026 09:53
Change-Id: I7c0b359c8241119f090b2092766e56fa0174464c
Change-Id: Ic43dffba4ad2fd78e78f5e70b16529069a034676
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

This PR disables the default "update on read" behavior in the BTBTAGE branch predictor configuration and refactors trace recording logic in the update function to source prediction metadata fields from the original prediction snapshot instead of recomputed values.

Changes

Cohort / File(s) Summary
Configuration Parameter
src/cpu/pred/BranchPredictor.py
Changed updateOnRead parameter default from True to False in BTBTAGE class configuration.
Trace Recording Logic
src/cpu/pred/btb/btb_tage.cc
Refactored BTBTAGE::update() to source trace fields (main_info, alt_info, useAlt, taken) from original prediction snapshot (trace_pred) rather than recomputed prediction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Restore/per table numways #798: Overlaps with refactoring of BTBTAGE::update() to source prediction fields from predMeta->preds instead of recomputed values.
  • Microtage merage  #775: Related through use of per-entry TageMeta preds and TAGE prediction metadata structures.

Suggested labels

align-kmhv3

Suggested reviewers

  • jensen-yan

Poem

🐰 The BTB TAGE hops along with grace,
No read-updates cluttering the place!
Traces now peek at the original sight,
Not recomputed dreams taking flight,
Branch predictions aligned just right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Tage trace fix' directly relates to the main changes in the pull request, which involve fixing trace record derivation in BTBTAGE::update method and adjusting the updateOnRead parameter.
Docstring Coverage ✅ Passed Docstring coverage is 100.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-trace-fix

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.

@CJ362ff CJ362ff requested a review from jensen-yan April 10, 2026 02:01
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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cpu/pred/btb/btb_tage.cc`:
- Around line 798-806: The code currently reads predMeta->preds[btb_entry.pc]
which default-inserts a trace prediction when the key is missing, mutating
predMeta and producing incorrect trace fields; replace that lookup with the
already-resolved source (use original_pred if present or recomputed if not)
instead of indexing predMeta->preds — i.e., obtain trace_pred from the existing
original_pred/recomputed variable used earlier and pass its fields into t.set
(keeping the same main_info/alt_info/taken/useAlt/alloc_success usage) so you
avoid implicit insertion into predMeta->preds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c0f1254-dc45-44dd-b24d-226aba928c4a

📥 Commits

Reviewing files that changed from the base of the PR and between abf85ff and 3e02cc5.

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

@github-actions
Copy link
Copy Markdown

🚀 Coremark Smoke Test Results

Branch IPC Change
Base (xs-dev) 2.2709 -
This PR 2.2713 📈 +0.0004 (+0.02%)

✅ Difftest smoke test passed!

@CJ362ff CJ362ff merged commit 52305c8 into xs-dev Apr 10, 2026
2 checks passed
@CJ362ff CJ362ff deleted the tage-trace-fix branch April 10, 2026 03:07
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.

2 participants