-
Notifications
You must be signed in to change notification settings - Fork 12.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
[RISCV] Cannot scavenge register without an emergency spill slot #58027
Comments
@llvm/issue-subscribers-backend-risc-v |
Reproduction 1 isn't really valid code. This GEP |
Reproducer 2 is also accessing past what is allocated. |
I agree that this code is invalid at memory level. However, it follows LLVM Language reference and is syntactically and semantically correct. Therefore, I would hope the backend have some proof to these ill-formed code instead of crashing. For example, in Reproduction 1, if we change index from 127 to 125, it is still invalid, but the compiler is handling it by ignoring invalid instructions. Our finding is that index |
I'm not sure I would say we crashed. We issued a fatal error because we didn't know how to proceed. It happens that fatal errors generate a stack trace by default. For index 125 we didn't need a scratch register to generate the immediates. We didn't ignore anything. We generated these accesses to out of bounds memory.
An index of 128 happens to get lucky and disabled a different optimization so we ended up putting part of the address in a7 and used small offset from it.
Exactly what indexes breaks is probably dependent on what other things end up on the stack from other allocations or from spills in register allocation. |
How about adding an emergency spill slot based on the stack size plus the memory instructions' offset? |
It's not clear we should increase compile time to scan all of the memory instructions to handle code that isn't valid. |
We think that https://reviews.llvm.org/D98101 will fix this issue and we're going to push forward on that. |
It seems that this has been fixed. Should we close it? |
I think the last commit of D98101 was 4554663 |
Description
For both RISCV32 and RISCV64, some specific value / range for the index argument of the
getelementptr
instruction may lead to crash with "Error while trying to spill X__ from class GPR: Cannot scavenge register without an emergency spill slot" when interacting with other instructions.Reproduction 1
In the following minimal case, the crash only seems to occur when the index argument for
getelementptr
is exactly 127.https://godbolt.org/z/seaPGnsY6
Code
Stack Trace
Reproduction 2
In the following case, the crashes occur when the index arguments for the two
getelementptr
are greater than some thresholds.https://godbolt.org/z/j6fK5K5G9
Code
Stack Trace
The text was updated successfully, but these errors were encountered: