-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
minor-cpu: unable to find destination due to unconditional branch followed by memory access #1130
Comments
Comment because it get lost in the text wall. |
This is a weird issue to happen on a in-order CPU. Is it possible this is a just RISCV issue (eg the instruction wrongly mislabelled) ? I am looking at the line:
And I don't understand why a jump instruction should be sent to the LSQ |
Hi, thank you for your comment 😄.
I do not think that the instruction is mislabelled, because at
In Lines 1686 to 1751 in 65b86cf
|
Hi @robhau
By mislabelled I don't mean mis-decoded. Basically in gem5 an instruction is defined (labelled) with some StaticInstFlag which are used by the CPU model to understand which kind of instruction it is dealing with (see https://github.com/gem5/gem5/blob/stable/src/cpu/StaticInstFlags.py). Is the instruction defined with IsUncondControl?
You are getting the condition wrong: Checks if there is a response in the LSQ only if the instruction is in the LSQ. So for whatever reason the Jump instruction gets added to the LSQ. This looks wrong to me. I would suggest you to debug the matter with GDB |
Hi @giactra
Thank you. The instructions are not mislabed. Jumps (32 Bit format): gem5/src/arch/riscv/isa/decoder.isa Lines 4878 to 4881 in 3b73071
gem5/src/arch/riscv/isa/decoder.isa Lines 4872 to 4876 in 3b73071
Compressed Jumps (16 bit format, used here): gem5/src/arch/riscv/isa/decoder.isa Lines 351 to 353 in 3b73071
gem5/src/arch/riscv/isa/decoder.isa Lines 215 to 218 in 3b73071
gem5/src/arch/riscv/isa/decoder.isa Lines 451 to 457 in 3b73071
gem5/src/arch/riscv/isa/decoder.isa Lines 471 to 474 in 3b73071
Thank you for the clarification Sorry, I quoted the wrong code section. Lines 1554 to 1571 in 3b73071
|
Could the branch target predictor be the problem? The branch predictor predicts taken, because it is an unconditional branch. gem5/src/cpu/pred/bpred_unit.cc Lines 120 to 163 in 3b73071
After that, the branch target buffer is looked up. However, there is a miss, because we this is the first time this jump is executed gem5/src/cpu/pred/bpred_unit.cc Lines 164 to 190 in 3b73071
gem5/src/cpu/pred/simple_btb.cc Lines 129 to 144 in 3b73071
Therefore, gem5/src/cpu/pred/bpred_unit.cc Lines 289 to 293 in 3b73071
Debug trace:
|
I thought about this at the beginning (wondering why an unconditional branch was predicted as not-taken). However the following reasoning convinced me there's something more to it:
My suspicion is that the load is issued before the branch commit, therefore before it can resteer the pipeline. While we should solve it (maybe be adding a similar check as the one we have in the O3CPU?) it seems to me a "niche" corner case. It only happens when virtual memory is disabled (at boot) and therefore we are allowed to send an invalid request down the memory subsystem. Normally (with virtual memory on) the MMU would return an exception to the translation request, and no memory reference would go out of the LSQ. Let me know if it makes sense to you |
Thank you @giactra, makes sense to me.
Adapting the handling from O3CPU sounds good. Can we add it right after inserting a decoded instruction into the output buffer of the decode stage?
Since we only carry out bare metal simulations, we run into this problem quite often 😅. |
Bug description
The program above can lead to an error when using MinorCPU. The reason is, that the memory access is started after the unconditional jump. If
a0
does not point to a valid memory region, an error is thrown inBaseXBar::findPort
.gem5/src/mem/xbar.cc
Lines 333 to 370 in b279e40
Affects version
Gem5 development branch, Commit SHA: b279e40
gem5 Modifications
No modifications.
To Reproduce
I pushed a minimal working example in https://github.com/robhau/gem5/tree/minor_cpu_bug_minimal_working_example
firmware/Makefile
bash execute.sh
Firmware code:
Makefile:
linker file:
disassembly of compiled firmware:
Terminal Output
Only with debug flag Exec
Debug flags Exec,Minor
Expected behavior
The memory access should be canceled after the unconditional jump.
If i remove
li a1, 1
fromstart:
, it works.Host Operating System
Ubuntu 22.04.4 LTS
Host ISA
X86
Compiler used
for gem5:
g++ version 11.4.0
linker: mold version 1.0.3
for firmware:
RISC-V GNU Compiler Toolchain (riscv64-unknown-elf-gcc version 13.2.0, GNU objdump version 2.42)
The text was updated successfully, but these errors were encountered: