Conversation
Change-Id: I07d3410e46b0229d925909a34c4d2620126e1d8f
…e in BTB implementations
Change-Id: Iae981456a53e24ea660e35f791176c4004c09531
WalkthroughThese changes enhance branch prediction statistics tracking across ITTAGE, MGSC, and RAS predictor components by adding new counters to track prediction hits, misses, and correctness outcomes. The commitBranch methods are updated to collect these metrics, and a potential use-after-free issue in ITTAGE is fixed by moving pred_npc extraction within proper bounds checking. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/cpu/pred/btb/btb_ittage.cc (1)
575-598: Use prefix increment operators for statistics counters.Static analysis correctly flags that
statistics::Scalaris a non-primitive type where prefix++operators are more efficient than postfix. Additionally, there's a minor formatting inconsistency at line 591.Apply this diff to use prefix operators and fix formatting:
// Update commit statistics if (this_branch_hit) { - ittageStats.commitHits++; + ++ittageStats.commitHits; if (pred_npc == npc) { - ittageStats.commitPredCorrect++; + ++ittageStats.commitPredCorrect; if (iscalled) { - ittageStats.callPredCorrect++; + ++ittageStats.callPredCorrect; } } else { - ittageStats.commitPredWrong++; + ++ittageStats.commitPredWrong; if (iscalled) { - ittageStats.callPredWrong++; + ++ittageStats.callPredWrong; } } if (iscalled) { - ittageStats.callHits++; + ++ittageStats.callHits; } - }else { - ittageStats.commitMisses++; - ittageStats.commitPredWrong++; + } else { + ++ittageStats.commitMisses; + ++ittageStats.commitPredWrong; if (iscalled) { - ittageStats.callMisses++; - ittageStats.callPredWrong++; + ++ittageStats.callMisses; + ++ittageStats.callPredWrong; } }src/cpu/pred/btb/btb_tage.cc (2)
1137-1153: Use prefix increment operators and fix formatting.Static analysis correctly flags that
statistics::Scalaris a non-primitive type where prefix++operators are more efficient. There's also a formatting inconsistency at line 1142.Apply this diff:
if (!predcorrect) { - tageStats.condPredwrong++; + ++tageStats.condPredwrong; if (!pred_hit) { - tageStats.condMissTakens++; + ++tageStats.condMissTakens; } - }else{ - tageStats.condCorrect++; + } else { + ++tageStats.condCorrect; if (!pred_hit) { - tageStats.condMissNoTakens++; + ++tageStats.condMissNoTakens; } } if (pred_hit) { - tageStats.predHit++; + ++tageStats.predHit; } else { - tageStats.predMiss++; + ++tageStats.predMiss; }
1139-1145: Consider clarifying statistic names for readability.The names
condMissTakensandcondMissNoTakenscould be misread as tracking taken/not-taken outcomes, but they actually track mispredictions vs correct predictions when there was no TAGE hit. Consider renaming for clarity.Suggested alternative names:
condMissTakens→condMispredOnMiss(misprediction when no hit)condMissNoTakens→condCorrectOnMiss(correct prediction when no hit)Or document the current names more explicitly in the stat descriptions.
src/cpu/pred/btb/ras.cc (3)
242-242: Consider using prefix increment for statistics counters.Static analysis suggests preferring prefix
++operators for non-primitive types. While the performance impact is negligible forstatistics::Scalar, prefix is generally preferred for consistency.- rasStats.Pushes++; + ++rasStats.Pushes;
286-286: Same here: prefer prefix increment.- rasStats.Pops++; + ++rasStats.Pops;
445-452: Prefer prefix increment for statistics counters.For consistency with best practices for non-primitive types.
- rasStats.PredWrong++; + ++rasStats.PredWrong; if (meta->sctr) { - rasStats.MispredWithSctr++; + ++rasStats.MispredWithSctr; } } else { - rasStats.PredCorrect++; + ++rasStats.PredCorrect; if (meta->sctr) { - rasStats.CorrectWithSctr++; + ++rasStats.CorrectWithSctr; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
configs/example/kmhv3.py(1 hunks)src/cpu/pred/btb/abtb.cc(0 hunks)src/cpu/pred/btb/btb_ittage.cc(2 hunks)src/cpu/pred/btb/btb_ittage.hh(1 hunks)src/cpu/pred/btb/btb_mgsc.cc(2 hunks)src/cpu/pred/btb/btb_mgsc.hh(1 hunks)src/cpu/pred/btb/btb_tage.cc(2 hunks)src/cpu/pred/btb/btb_tage.hh(1 hunks)src/cpu/pred/btb/mbtb.cc(0 hunks)src/cpu/pred/btb/ras.cc(4 hunks)src/cpu/pred/btb/ras.hh(1 hunks)
💤 Files with no reviewable changes (2)
- src/cpu/pred/btb/mbtb.cc
- src/cpu/pred/btb/abtb.cc
🧰 Additional context used
🧬 Code graph analysis (5)
configs/example/kmhv3.py (1)
src/cpu/pred/btb/test/abtb.test.cc (1)
abtb(59-67)
src/cpu/pred/btb/btb_ittage.cc (1)
src/cpu/pred/btb/btb_ittage.hh (1)
stream(114-114)
src/cpu/pred/btb/btb_mgsc.cc (3)
src/cpu/pred/btb/btb_tage.cc (2)
commitBranch(1119-1154)commitBranch(1120-1120)src/cpu/pred/btb/btb_ubtb.cc (2)
commitBranch(313-402)commitBranch(314-314)src/cpu/pred/btb/btb_mgsc.hh (11)
stream(162-162)stream(258-258)pc(171-171)pc(216-216)pc(221-221)pc(228-228)pc(231-231)pc(234-234)pc(234-234)pc(248-251)pc(248-248)
src/cpu/pred/btb/btb_tage.cc (3)
src/cpu/pred/btb/btb_mgsc.hh (2)
stream(162-162)stream(258-258)src/cpu/pred/btb/btb_tage.hh (3)
stream(155-155)stream(277-277)stream(418-418)src/cpu/pred/ftb/ftb_tage.hh (1)
stream(98-98)
src/cpu/pred/btb/ras.cc (1)
src/cpu/pred/btb/ras.hh (4)
stream(107-107)stream(109-109)stream(112-112)RASStats(237-237)
🪛 Cppcheck (2.18.0)
src/cpu/pred/btb/btb_ittage.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 576-576: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 578-578: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 580-580: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 583-583: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 585-585: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 589-589: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 592-592: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 593-593: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 595-595: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 596-596: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/cpu/pred/btb/btb_mgsc.cc
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 1118-1118: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1120-1120: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1122-1122: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1125-1125: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1127-1127: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1131-1131: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1133-1133: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1134-1134: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1136-1136: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1139-1139: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1140-1140: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1142-1142: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/cpu/pred/btb/btb_tage.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 1138-1138: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1140-1140: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1143-1143: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1145-1145: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1150-1150: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 1152-1152: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/cpu/pred/btb/ras.cc
[information] 54-54: Include file
(missingIncludeSystem)
[information] 55-55: Include file
(missingIncludeSystem)
[information] 54-54: Include file
(missingInclude)
[information] 54-54: Include file
(missingIncludeSystem)
[information] 55-55: Include file
(missingIncludeSystem)
[information] 54-54: Include file
(missingInclude)
[information] 54-54: Include file
(missingInclude)
[information] 55-55: Include file
(missingInclude)
[information] 55-55: Include file
(missingInclude)
[information] 55-55: Include file
(missingIncludeSystem)
[information] 55-55: Include file
(missingInclude)
[information] 54-54: Include file
(missingInclude)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 55-55: Function 'getNonSpecStack()' should return member 'nonSpecStack' by const reference.
(returnByReference)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 242-242: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 286-286: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 438-438: Variable 'preds' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 439-439: Variable 'indexBwFoldedHist' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 440-440: Variable 'indexLFoldedHist' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 441-441: Variable 'indexIFoldedHist' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 442-442: Variable 'indexGFoldedHist' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 443-443: Variable 'indexPFoldedHist' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
[performance] 445-445: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 447-447: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 450-450: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[performance] 452-452: Prefer prefix ++/-- operators for non-primitive types.
(postfixOperator)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: perf_test / XS-GEM5 - Run performance test (spec06-0.3c)
🔇 Additional comments (14)
configs/example/kmhv3.py (1)
93-109: ABTB enablement in KMH v3 branch predictor config looks consistentTurning
cpu.branchPred.abtb.enabledtoTrueunder theDecoupledBPUWithBTBconfiguration is consistent with the surrounding predictor setup (ubtb,mbtb,tageenabled;ittage,mgsc,rasdisabled). I don’t see configuration or wiring issues in this block.src/cpu/pred/btb/btb_ittage.hh (1)
220-228: LGTM! New commit-time statistics members added.The eight new statistics members provide comprehensive tracking for ITTAGE commit behavior, covering hits/misses and prediction correctness for both general indirect branches and calls specifically.
src/cpu/pred/btb/btb_ittage.cc (2)
558-561: LGTM! Proper filtering for indirect non-return branches.The early return correctly scopes ITTAGE statistics to only indirect control flow instructions that are not returns, matching the predictor's purpose.
616-625: LGTM! Statistics properly initialized.The new commit-time statistics are correctly registered with descriptive labels following the existing pattern.
src/cpu/pred/btb/btb_tage.cc (2)
1032-1039: LGTM! New conditional branch statistics properly registered.The six new statistics for tracking conditional branch commit behavior are correctly initialized with appropriate descriptions.
1122-1125: LGTM! Proper filtering for conditional branches.The early return correctly scopes TAGE statistics to only conditional control flow instructions, matching the predictor's purpose.
src/cpu/pred/btb/btb_tage.hh (1)
366-372: LGTM! New conditional branch statistics members added.The six new statistics members properly extend TageStats to track commit-time behavior for conditional branches, consistent with the parallel additions in ITTAGE.
src/cpu/pred/btb/btb_mgsc.hh (1)
392-399: LGTM!The new statistics counters are well-organized and follow the existing pattern in the struct. The naming clearly indicates their purpose for tracking commit-time prediction statistics.
src/cpu/pred/btb/btb_mgsc.cc (2)
3-6: LGTM!The
UNIT_TESTguard allows compiling this module without the full O3 CPU dependencies during unit testing, which is a good practice for testability.
1085-1094: LGTM!The new statistics are properly initialized with appropriate descriptions. They align with the header declarations and follow the existing pattern.
src/cpu/pred/btb/ras.hh (1)
215-219: LGTM on the Scalar typedef approach.The conditional typedef pattern cleanly abstracts away the difference between test and production statistics types.
src/cpu/pred/btb/ras.cc (3)
54-55: LGTM on constructor initialization.Properly initializes
rasStatswiththisto register the statistics group with the parent.
435-455: LGTM on the commitBranch implementation.The logic correctly:
- Filters to only return instructions (excluding NOPs) at line 438.
- Retrieves the prediction metadata and actual next PC.
- Compares predicted target against actual NPC to determine prediction correctness.
- Tracks the
sctr > 0correlation for deeper analysis of mispredictions with counter-compressed stack entries.
457-470: LGTM on RASStats constructor.The statistics are properly registered using
ADD_STATwith appropriate units and descriptions.
|
[Generated by GEM5 Performance Robot] Ideal BTB PerformanceOverall Score
[Generated by GEM5 Performance Robot] Ideal BTB PerformanceOverall Score
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cpu/pred/btb/btb_ittage.cc (1)
574-578: Guardedpred_npcassignment removes the UB; optional clarity tweakMoving the
pred_npcassignment behind thepred_it != meta->preds.end()check correctly avoids dereferencingend()and fixes the prior undefined behavior. As a small readability/diagnostics tweak, you might want to initializepred_npc(e.g.,Addr pred_npc = 0;) to make it obviously safe even if the compiler can’t prove thethis_branch_hitguard, but functionally this is fine as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cpu/pred/btb/btb_ittage.cc(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cpu/pred/btb/btb_ittage.cc (1)
src/cpu/pred/btb/btb_ubtb.hh (1)
meta(144-147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Quick Build, Unit Tests & Smoke Test
- GitHub Check: perf_test / XS-GEM5 - Run performance test (spec06-0.3c)
🔇 Additional comments (2)
src/cpu/pred/btb/btb_ittage.cc (2)
582-605: Confirm intended semantics for*_PredWrongon ITTAGE missesRight now,
commitPredWrong(andcallPredWrong) is incremented both when:
- there is an ITTAGE hit but the predicted target (
pred_npc) is wrong, and- there is no ITTAGE prediction at all (miss branch).
That makes
commitPredCorrect + commitPredWrongequal the total number of indirect (and call) commits, whilecommitHits + commitMissestracks presence/absence of an ITTAGE entry. If instead you want “*_PredWrong” to count only cases where ITTAGE actually provided a prediction that turned out wrong (excluding pure misses), you’d drop the*_PredWrongincrements from theelse(miss) path and rely solely on*_Missesthere.Please double-check which definition you want for downstream analysis and adjust accordingly if needed.
625-632: New commit/call stats wiring looks consistentThe added
commitHits/Misses,callHits/Misses, andcommitPred*/callPred*counters are cleanly registered and match the updates incommitBranch, so the stats plumbing itself looks correct.
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
3 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.