Skip to content

Sc test align #852

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 9 commits into
base: xs-dev
Choose a base branch
from
Open

Sc test align #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
56c96f0
configs: Disable MGSC tables for CI ablation
jensen-yan May 7, 2026
0ad83c5
configs: Enable only MGSC global table
jensen-yan May 7, 2026
4b8bcab
configs: Enable only MGSC local table
jensen-yan May 7, 2026
44ea6ae
configs: Enable only MGSC IMLI table
jensen-yan May 7, 2026
fc42281
configs: Enable only MGSC backward table
jensen-yan May 7, 2026
0809aa0
configs: Enable only MGSC path table
jensen-yan May 7, 2026
f56da4a
configs: Enable only MGSC bias table
jensen-yan May 7, 2026
65f9222
bpu: Tighten MGSC override gating
jensen-yan May 8, 2026
847b2e4
configs: Enable only MGSC global table
jensen-yan May 8, 2026
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
7 changes: 7 additions & 0 deletions configs/example/kmhv3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ def setKmhV3Params(args, system):
cpu.branchPred.tage.enabled = True
cpu.branchPred.ittage.enabled = True
cpu.branchPred.mgsc.enabled = True
cpu.branchPred.mgsc.enableBwTable = False
cpu.branchPred.mgsc.enableLTable = False
cpu.branchPred.mgsc.enableITable = False
cpu.branchPred.mgsc.enableGTable = True
cpu.branchPred.mgsc.enablePTable = False
cpu.branchPred.mgsc.enableBiasTable = False
cpu.branchPred.mgsc.enablePCThreshold = True
cpu.branchPred.ras.enabled = True

# l1 cache per core
Expand Down
10 changes: 5 additions & 5 deletions src/cpu/pred/btb/btb_mgsc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,15 +432,15 @@ BTBMGSC::generateSinglePrediction(const BTBEntry &btb_entry, const Addr &startPC
bool use_sc_pred = forceUseSC; // Force use SC if configured
if (!use_sc_pred) {
if (tage_info.tage_pred_conf_high) {
if (abs(total_sum) > total_thres / 2) {
if (abs(total_sum) > total_thres) {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 8, 2026

Choose a reason for hiding this comment

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

P1 Badge Update the high-confidence MGSC gate test vector

With the unchanged BTBMGSCTest.GateHighConfUsesSCWhenStrong vector, total_sum is 18 and the default total_thres is 35. After this threshold bump, high-confidence predictions require 18 > 35, so use_mgsc stays false and the prediction falls back to TAGE/not-taken instead of the test's expected SC/taken result; please update the unit test inputs/expectations along with the new gate semantics so the MGSC test suite does not fail.

Useful? React with 👍 / 👎.

use_sc_pred = true;
}
} else if (tage_info.tage_pred_conf_mid) {
if (abs(total_sum) > total_thres / 4) {
if (abs(total_sum) > total_thres / 2) {
use_sc_pred = true;
}
} else if (tage_info.tage_pred_conf_low) {
if (abs(total_sum) > total_thres / 8) {
if (abs(total_sum) > total_thres / 4) {
use_sc_pred = true;
}
}
Expand Down Expand Up @@ -792,8 +792,8 @@ BTBMGSC::updateSinglePredictor(const BTBEntry &entry, bool actual_taken, const M
#ifndef UNIT_TEST
// Write trace record
if (enableDB && (focusBranchPC == 0 || entry.pc == focusBranchPC)) {
auto effective_gate = pred.tage_conf_high ? (total_thres / 2)
: (pred.tage_conf_mid ? (total_thres / 4) : (total_thres / 8));
auto effective_gate = pred.tage_conf_high ? total_thres
: (pred.tage_conf_mid ? (total_thres / 2) : (total_thres / 4));
auto margin = std::abs(total_sum) - effective_gate;
Comment on lines +795 to 797
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

effective_gate defaults to total_thres/4 when no confidence flag is set

The ternary chain falls through to total_thres / 4 when all three conf flags (tage_conf_high, tage_conf_mid, tage_conf_low) are false — e.g. when forceUseSC = true with no conf flag asserted, or any future code path where the flags are all clear. In that state the logged margin is computed against a phantom gate value, making the trace misleading for those rows.

🐛 Proposed fix — mirror the prediction-side `if/else if` structure 
-        auto effective_gate = pred.tage_conf_high ? total_thres
-            : (pred.tage_conf_mid ? (total_thres / 2) : (total_thres / 4));
+        auto effective_gate = pred.tage_conf_high   ? total_thres
+            : pred.tage_conf_mid                    ? (total_thres / 2)
+            : pred.tage_conf_low                    ? (total_thres / 4)
+            : 0;  // no confidence tier active (e.g. forceUseSC); gate is irrelevant
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
auto effective_gate = pred.tage_conf_high ? total_thres
: (pred.tage_conf_mid ? (total_thres / 2) : (total_thres / 4));
auto margin = std::abs(total_sum) - effective_gate;
auto effective_gate = pred.tage_conf_high ? total_thres
: pred.tage_conf_mid ? (total_thres / 2)
: pred.tage_conf_low ? (total_thres / 4)
: 0; // no confidence tier active (e.g. forceUseSC); gate is irrelevant
auto margin = std::abs(total_sum) - effective_gate;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cpu/pred/btb/btb_mgsc.cc` around lines 795 - 797, The ternary used to
compute effective_gate can fall through to total_thres/4 when no confidence
flags are set, producing a misleading margin; change the calculation to mirror
the prediction-side if/else if structure: test pred.tage_conf_high, then else if
pred.tage_conf_mid, then else if pred.tage_conf_low, and only if none are set
use a sensible default (e.g. treat as no gate or skip logging) before computing
margin = std::abs(total_sum) - effective_gate; update the logic around
effective_gate, margin, and any code paths involving forceUseSC so margin
reflects the same gate selection rules as the predictor.

auto foldIndexSig = [](const std::vector<unsigned> &indices) -> uint64_t {
uint64_t sig = 0xcbf29ce484222325ULL;
Expand Down
Loading