Fix RAS pop-then-push writing the new return address to the wrong stack slot#31
Open
tejassinghbhati wants to merge 3 commits into
Open
Fix RAS pop-then-push writing the new return address to the wrong stack slot#31tejassinghbhati wants to merge 3 commits into
tejassinghbhati wants to merge 3 commits into
Conversation
For JALR with rd=link, rs1=link, rd!=rs1 the decoder asserts push and pop simultaneously (pop-then-push per the RISC-V RAS hint table). The RAS wrote the new return address at head_pointer, the empty slot above the top of the stack, leaving the stack unchanged and dropping the new return address. The following return then always mispredicted. Write the replacement into the top entry (output_pointer = head_pointer - 1) instead. The head pointer correctly stays unchanged, as pop-then-push leaves the stack depth the same. Fixes bsc-loca#30
Covers basic LIFO ordering, the pop-then-push top-entry replacement (regression for the bug fixed in the previous commit), chained pop-then-push sequences, and full-depth pointer wrap-around. Follows the structure of the neighboring tb_branch_predictor testbench and is wired into scripts/questa_ci.sh. Verified with Verilator 5.032 (--binary --timing): all 25 checks pass on the fixed RTL; against the previous RTL the pop-then-push checks fail (stale top-of-stack value returned), confirming the regression isolates the defect described in bsc-loca#30.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JALRwithrd = link,rs1 = link,rd != rs1, e.g.jalr ra, t0) replaces the top-of-stack entry instead of writing to the dead slot above itreturn_address_stack.sv(there was none) covering LIFO ordering, pop-then-push top-entry replacement, chained pop-then-push, and full-depth pointer wrap-around, wired intoscripts/questa_ci.shWhy
head_pointerpoints to the next free slot; the top of the stack is read fromoutput_pointer = head_pointer - 1. The simultaneouspush_i && pop_icase wroteaddress_stack[head_pointer], a slotreturn_address_onever reads and the nextplain push overwrites. The stack was therefore left unchanged and the new return
address was dropped, so the following
retin that call chain always mispredicted.See #30 for the full walk-through.
Fix
head_pointercorrectly stays unchanged, since pop-then-push leaves the stackdepth the same. Performance-only; no architectural behavior change.
Validation
Ran the new testbench with Verilator 5.032 (
--binary --timing):fixed RTL: all 25 checks pass (
TEST PASSED)original RTL (negative control): the pop-then-push regression checks fail
exactly as the issue predicts, while plain LIFO and wrap-around still pass:
The testbench follows the structure of the neighboring
tb_branch_predictor(same
runtest.shQuestaSim flow,colors.vh,wave.do) so it also runs undermake questaviascripts/questa_ci.sh.Fixes #30