Skip to content

bpu: test tage not use alt table #690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jensen-yan wants to merge 1 commit into
base: xs-dev
Choose a base branch
from
Open

bpu: test tage not use alt table #690

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 24 additions & 33 deletions src/cpu/pred/btb/btb_tage.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ BTBTAGE::generateSinglePrediction(const BTBEntry &btb_entry,
std::shared_ptr<TageMeta> predMeta) {
DPRINTF(TAGE, "generateSinglePrediction for btbEntry: %#lx\n", btb_entry.pc);

// Find main and alternative predictions
// Find main prediction only (alt provider disabled).
bool provided = false;
bool alt_provided = false;
TageTableInfo main_info, alt_info;
TageTableInfo main_info;
TageTableInfo alt_info; // kept for metadata/DB schema compatibility; always empty.

// Search from highest to lowest table for matches
// Calculate branch position within the block (like RTL's cfiPosition)
Expand Down Expand Up @@ -248,34 +248,31 @@ BTBTAGE::generateSinglePrediction(const BTBEntry &btb_entry,
}

if (match) {
if (!provided) {
// First match becomes main prediction
main_info = TageTableInfo(true, matching_entry, i, index, tag, matching_way);
provided = true;
} else if (!alt_provided) {
// Second match becomes alternative prediction
alt_info = TageTableInfo(true, matching_entry, i, index, tag, matching_way);
alt_provided = true;
break;
}
// First match becomes main prediction; alt provider is disabled
main_info = TageTableInfo(true, matching_entry, i, index, tag, matching_way);
provided = true;
break;
} else {
DPRINTF(TAGE, "miss table %d[%lu] for tag %lu (with pos %u), btb_pc %#lx\n",
i, index, tag, position, btb_entry.pc);
}
}

// Generate final prediction
bool main_taken = main_info.taken();
bool alt_taken = alt_info.taken();
// Use base table instead of btb_entry.ctr
const bool main_taken = main_info.taken();
// Use base table instead of btb_entry.ctr.
Addr base_idx = getBaseTableIndex(startPC);
unsigned branch_idx = getBranchIndexInBlock(btb_entry.pc, startPC);
bool base_taken = getDelay() != 0 ? (usingBasetable ? baseTable[base_idx][branch_idx] >= 0 : btb_entry.ctr >= 0)
: btb_entry.ctr >= 0;
//bool base_taken = btb_entry.ctr >= 0;
bool alt_pred = alt_provided ? alt_taken : base_taken; // if alt provided, use alt prediction, otherwise use base
const bool base_taken =
getDelay() != 0 ?
(usingBasetable ? baseTable[base_idx][branch_idx] >= 0 : btb_entry.ctr >= 0) :
btb_entry.ctr >= 0;

// With alt provider disabled, "alt" always means the base fallback.
const bool alt_taken = base_taken;
const bool alt_pred = base_taken;

// use_alt_on_na gating: when provider weak, consult per-PC counter
// With alt disabled, allow use_alt_on_na to choose base vs main when main is weak.
bool use_alt = false;
if (!provided) {
use_alt = true;
Expand All @@ -291,8 +288,8 @@ BTBTAGE::generateSinglePrediction(const BTBEntry &btb_entry,
bool taken = use_alt ? alt_pred : main_taken;

DPRINTF(TAGE, "tage predict %#lx taken %d\n", btb_entry.pc, taken);
DPRINTF(TAGE, "tage use_alt %d ? (alt_provided %d ? alt_taken %d : base_taken %d) : main_taken %d\n",
use_alt, alt_provided, alt_taken, base_taken, main_taken);
DPRINTF(TAGE, "tage use_alt %d ? base_taken %d : main_taken %d\n",
use_alt, base_taken, main_taken);

return TagePrediction(btb_entry.pc, main_info, alt_info, use_alt, taken, alt_pred);
}
Expand Down Expand Up @@ -442,7 +439,8 @@ BTBTAGE::updatePredictorStateAndCheckAllocation(const BTBEntry &entry,
Addr base_idx = getBaseTableIndex(startPC);
unsigned branch_idx = getBranchIndexInBlock(entry.pc, startPC);
bool base_taken = baseTable[base_idx][branch_idx] >= 0;
bool alt_taken = alt_info.found ? alt_info.taken() : base_taken;
// Alt provider disabled: "alt" is always the base fallback.
const bool alt_taken = base_taken;
Comment thread
jensen-yan marked this conversation as resolved.
Comment on lines 439 to +443

@coderabbitai coderabbitai Bot Jan 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix inconsistent base_taken calculation in update path.

The update path recalculates base_taken using simplified logic that always reads from baseTable (line 441), but the prediction path uses conditional logic based on getDelay() and usingBasetable (lines 266-269):

// Prediction path (lines 266-269):
const bool base_taken =
    getDelay() != 0 ?
        (usingBasetable ? baseTable[base_idx][branch_idx] >= 0 : btb_entry.ctr >= 0) :
        btb_entry.ctr >= 0;

// Update path (line 441):
bool base_taken = baseTable[base_idx][branch_idx] >= 0;  // ← always baseTable

This inconsistency means the update path may determine alt_correct (line 451) based on a different prediction than what was actually made, leading to incorrect use_alt_on_na counter updates and statistics.

Solution: Use pred.altPred which already contains the correctly calculated prediction from generateSinglePrediction:

🔎 Proposed fix 
-    // Use base table instead of entry.ctr for fallback prediction
-    Addr startPC = stream.getRealStartPC();
-    Addr base_idx = getBaseTableIndex(startPC);
-    unsigned branch_idx = getBranchIndexInBlock(entry.pc, startPC);
-    bool base_taken = baseTable[base_idx][branch_idx] >= 0;
-    // Alt provider disabled: "alt" is always the base fallback.
-    const bool alt_taken = base_taken;
+    // Alt provider disabled: use pred.altPred which was calculated in generateSinglePrediction
+    const bool alt_taken = pred.altPred;

This ensures consistency between prediction and update, and you can remove the now-unused base_idx and branch_idx calculations from lines 439-440.

🤖 Prompt for AI Agents 
In src/cpu/pred/btb/btb_tage.cc around lines 439-443, the update path recomputes
base_taken directly from baseTable which is inconsistent with the prediction
path that considers getDelay() and usingBasetable; replace the recomputed
base/alt logic by using the already-computed pred.altPred (and/or pred.basePred
as appropriate) from generateSinglePrediction for determining base_taken and
alt_taken, and remove the now-unused base_idx and branch_idx calculations so the
update uses the exact prediction made during the prediction phase.


// Update use_alt_on_na when provider is weak (0 or -1)
if (main_info.found) {
Expand Down Expand Up @@ -473,9 +471,7 @@ BTBTAGE::updatePredictorStateAndCheckAllocation(const BTBEntry &entry,

// Update useful bit based on several conditions
bool main_is_correct = main_info.taken() == actual_taken;
bool alt_is_correct_and_strong = alt_info.found &&
(alt_info.taken() == actual_taken) &&
(abs(2 * alt_info.entry.counter + 1) == 7);
const bool alt_is_correct_and_strong = false; // no alt provider

// a. Special reset (humility mechanism)
if (alt_is_correct_and_strong && main_is_correct) {
Expand All @@ -498,12 +494,7 @@ BTBTAGE::updatePredictorStateAndCheckAllocation(const BTBEntry &entry,
// No LRU maintenance
}

// Update alternative prediction provider
if (used_alt && alt_info.found) {
auto &way = tageTable[alt_info.table][alt_info.index][alt_info.way];
updateCounter(actual_taken, 3, way.counter);
// No LRU maintenance
}
// Alt provider disabled: no alternative provider counter updates.

// Update base table counter if used as fallback
if (used_alt && !alt_info.found) {
Expand Down
Loading