ZJIT: Panic unimplemented for OOB basic block args #13533
Conversation
zjit/src/backend/x86_64/mod.rs
Outdated
| @@ -90,6 +90,8 @@ pub const ALLOC_REGS: &'static [Reg] = &[ | |||
| R9_REG, | |||
| R10_REG, | |||
| RAX_REG, | |||
| R14_REG, | |||
| R15_REG, | |||
There was a problem hiding this comment.
r14 and r15 are callee-saved registers. The register allocator currently doesn't support callee-saved registers, so they're deliberately not added to the list at the moment. This branch could cause an undefined behavior depending on a C function called in JIT code. We'd need to preserve them when entering/leaving JIT code, but that's also overhead we need to carefully handle.
So our path forward is probably to support register spill first, before expanding the register pool to callee-saved registers.
zjit/src/codegen.rs
Outdated
| use super::*; | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "register spilling not yet implemented, too many basic block arguments (11/10)")] |
There was a problem hiding this comment.
The panic (by unimplemented!) itself works as an inline test, so I don't think you should test the test. It's also tedious to update the "11/10" part whenever you touch the ALLOC_REGS constant.
f5d682f to
3a5e260
Compare
|
Thank you for explaining why I was in error, @k0kubun. I very much appreciate your thoughtful response. I've updated the PR accordingly, removing the new registers and unnecessary test. I updated this PR to only include the unimplemented panic message. |
ALLOC_REGSregisters for ARM64 and x86_64Testing ZJIT I noticed an index out of bounds panic with:
ruby --zjit -e "pp 0"Pending the implementation of register spilling, I propose: