Conversation
Change-Id: I04dee1db8d1dab65dca9ff775d7128e7cee33ed8
Change-Id: I608fe975cf564882034081f632cbf72ef1d78181
📝 WalkthroughWalkthroughUpdated MicroTAGE public parameters in the branch predictor: tag bit sizes reduced, history lengths changed, associativity increased, and a base table size parameter removed; small formatting adjustments only. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Change-Id: I97523067334a8c2612684d1642149a1fad0d12c1
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
Change-Id: Iefa296f28a6561a7251abc96e96392239d920de1
Change-Id: I6b0fea7c8b29ecef17b761697506a83dbd0de500
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cpu/pred/BranchPredictor.py (1)
1082-1082: Please cover the new 2-way MicroTAGE path.Line 1082 now enables
way == 1allocation/update behavior insrc/cpu/pred/btb/microtage.cc:518-527. A focused regression or config that forces second-way hits/replacement would make this tuning safer to land.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cpu/pred/BranchPredictor.py` at line 1082, Branch predictor changes enable the MicroTAGE two-way path (allocation/update for way == 1) but there's no focused test exercising it; add a small regression/config that forces second-way hits and replacements to validate the new path. Create a unit/regression harness that instantiates the BranchPredictor/MicroTAGE with numWays=2 (the Param.Unsigned setting in BranchPredictor.py) and run a synthetic branch stream that causes collisions in the BTB to trigger allocations/updates on way==1 (the code path in microtage.cc handling way == 1); ensure the test asserts expected predictions/updates and fails if way==1 never executes so the two-way behavior is covered.
🤖 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/BranchPredictor.py`:
- Line 1082: Branch predictor changes enable the MicroTAGE two-way path
(allocation/update for way == 1) but there's no focused test exercising it; add
a small regression/config that forces second-way hits and replacements to
validate the new path. Create a unit/regression harness that instantiates the
BranchPredictor/MicroTAGE with numWays=2 (the Param.Unsigned setting in
BranchPredictor.py) and run a synthetic branch stream that causes collisions in
the BTB to trigger allocations/updates on way==1 (the code path in microtage.cc
handling way == 1); ensure the test asserts expected predictions/updates and
fails if way==1 never executes so the two-way behavior is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7df1b22-069c-4f7b-982b-64d85e045d02
📒 Files selected for processing (1)
src/cpu/pred/BranchPredictor.py
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
|
[Generated by GEM5 Performance Robot] Align BTB PerformanceOverall Score
|
Summary by CodeRabbit