ZJIT: Add MemBase::FrameBase that deals with native SP changes (+2) #14009
Conversation
Keeping the same name makes re-exporting more concise.
This is to fix `def a(n1,n2,n3,n4,n5,n6,n7,n8) = [n8]`, and for future changes that push values onto the native stack. Before this, the backend automatically preserved the array in gen_new_array() with an inserted `asm.c_push()`, which invalidated the static memory offset based on native SP for `n8`.
|
I like high watermark and bump SP (with RBP) approach personally |
If we always allocate
The problem Alan is talking about is that the backend moves SP on its own for pushing live registers on C calls. To address that with a high watermark, you need to know the maximum number of live registers on C calls. It's impossible to know as of setting up a frame, so I guess we'd have to always bump SP by the number of allocatable registers. I wonder how much stack space we can safely waste for each frame. |
Ah yes, that's another approach. If we're willing to preserve RBP on x86 (we don't currently) then we can push and pop and refer to things using a static offset based on RBP. On ARM we already always have an RBP equivalent because macOS requires it. |
|
Sorry for misunderstanding the idea. So... I guess we'll tweak the FrameSetup for x86_64 and switch to RBP-based operands then? |
❌ Tests Failed✖️no tests failed ✔️62162 tests passed(1 flake) |
Yes that's the idea. Seems like the best option for now. |
This solves an immediate problem with
def a(n1,n2,n3,n4,n5,n6,n7,n8) = [n8]and gen_new_array(). In the prior, it miscomps as follows:The problem here is that n8 is assigned a fixed offset from sp in codegen.rs, but the backend can move sp. To solve this, this diff adds
MemBase::FrameBase, which keeps track of pushes and pops so code can refer to the pre-modification sp (base sp, as I call it in the diff). The tracking has to be done late in the backend because the backend inserts pushes and pops, like we see here.This will also be useful for things like
concatstrings, where we want to generate LIR that pushes onto the native stack and still refer to other on stack elements afterwards.Now, I'm not a huge fan of this because this feels like a leaky abstraction. It can only track compile-time known modifications to SP like stack pushes and pops. Dynamic stack space allocation will silently miscomp. The tracking is also sensitive to the sequence of LIR that ends up modifying SP, so compile time knowable modifications might not always be properly tracked if they're too indirect.
What to do then?
Some quick possible alternatives:
I'd like your opinion on this.