Skip to content

Commit 6812609

Browse files
committed
Fix getIP on aarch64: SP-relative saved-IP slot [SP,#8]
Save JIT return address to [SP, #8] before each BL/BLR instruction, and read it back in getIP() at frame_base - frame_size - kPointerSize. Falls back to saved LR at [FP+8] when slot reads zero (no call yet). Root causes fixed: 1. Double-counting: old FP-relative STR offset added kPointerSize on top of stack_frame_size which already includes kStackAlign, writing below SP. 2. Register clobbering: translateCall isReg case could have call target in x12/x13 which saveReturnAddress overwrote. Now moves target to x16 first. 3. ptr_resolve scratch: large frame offsets triggered ptr_resolve paths using x13 as scratch, clobbering the call target. SP-relative [SP,#8] uses simple immediate addressing with no scratch register. Validated: 15/15 CPU modules pass (default + PYTHONJITLISTALL=1), 8/8 GPU modules pass with zero regressions, GPU JIT smoke test pass (add/matmul/relu on NVIDIA GB200).
1 parent 74111fa commit 6812609

4 files changed

Lines changed: 38 additions & 24 deletions

File tree

cinderx/Jit/codegen/autogen.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1937,20 +1937,35 @@ void translateCall(Environ* env, const Instruction* instr) {
19371937
auto output = instr->output();
19381938
auto input = instr->getInput(0);
19391939

1940+
// Save return address to [SP, #8] before BLR, matching x86 call semantics.
1941+
// Move call target to x16 first in case input register is x12 (scratch_0).
1942+
asmjit::Label after_call = as->newLabel();
1943+
19401944
if (input->isReg()) {
1941-
as->blr(AT::getGp(input));
1945+
auto target = AT::getGp(input);
1946+
if (target.id() != arch::reg_scratch_br.id()) {
1947+
as->mov(arch::reg_scratch_br, target);
1948+
}
1949+
as->adr(arch::reg_scratch_0, after_call);
1950+
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
1951+
as->blr(arch::reg_scratch_br);
19421952
} else if (input->isImm()) {
19431953
as->mov(arch::reg_scratch_br, input->getConstant());
1954+
as->adr(arch::reg_scratch_0, after_call);
1955+
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
19441956
as->blr(arch::reg_scratch_br);
19451957
} else if (input->isStack()) {
19461958
auto loc = input->getStackSlot().loc;
19471959
as->ldr(
19481960
arch::reg_scratch_br,
19491961
arch::ptr_resolve(as, arch::fp, loc, arch::reg_scratch_0));
1962+
as->adr(arch::reg_scratch_0, after_call);
1963+
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
19501964
as->blr(arch::reg_scratch_br);
19511965
} else {
19521966
JIT_ABORT("Unsupported operand type for Call: {}", input->type());
19531967
}
1968+
as->bind(after_call);
19541969

19551970
if (instr->origin()) {
19561971
asmjit::Label label = as->newLabel();

cinderx/Jit/codegen/gen_asm.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,9 @@ int NativeGenerator::allocateHeaderAndSpillSpace(const FrameInfo& frame_info) {
14571457
as_->sub(a64::sp, a64::sp, arch::reg_scratch_0);
14581458
}
14591459
}
1460+
// Zero-init the saved-IP slot at [SP, #8]. getIP() falls back to
1461+
// [FP+8] (saved LR) when this slot reads 0.
1462+
as_->str(a64::xzr, asmjit::arm::Mem(a64::sp, 8));
14601463

14611464
// There is a difference here from x86-64, because the aarch64 stack cannot be
14621465
// misaligned. Here we are returning the amount of space that we have added to

cinderx/Jit/codegen/gen_asm_utils.cpp

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,13 @@ void emitCall(
2626
#if defined(CINDER_X86_64)
2727
env.as->call(label);
2828
#elif defined(CINDER_AARCH64)
29-
// Save return address to stack before bl, matching x86 call semantics.
30-
// Slot at [FP - (stack_frame_size - 8)] = [SP + 8], within the extra
31-
// kStackAlign bytes reserved by computeFrameInfo.
29+
// Save return address to [SP, #8] before BL, matching x86 call semantics.
30+
// Slot is within the kStackAlign extra bytes reserved by computeFrameInfo.
31+
// Using SP-relative avoids ptr_resolve scratch register issues.
3232
{
3333
asmjit::Label after_call = env.as->newLabel();
34-
int offset = -(env.stack_frame_size - kPointerSize);
3534
env.as->adr(arch::reg_scratch_0, after_call);
36-
env.as->str(
37-
arch::reg_scratch_0,
38-
arch::ptr_resolve(env.as, arch::fp, offset, arch::reg_scratch_1));
35+
env.as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
3936
env.as->bl(label);
4037
env.as->bind(after_call);
4138
}
@@ -54,14 +51,11 @@ void emitCall(Environ& env, uint64_t func, const jit::lir::Instruction* instr) {
5451
// https://github.com/asmjit/asmjit/issues/499, but as of writing is not yet
5552
// available.
5653
env.as->mov(arch::reg_scratch_br, func);
57-
// Save return address to stack before blr, matching x86 call semantics.
54+
// Save return address to [SP, #8] before BLR.
5855
{
5956
asmjit::Label after_call = env.as->newLabel();
60-
int offset = -(env.stack_frame_size - kPointerSize);
6157
env.as->adr(arch::reg_scratch_0, after_call);
62-
env.as->str(
63-
arch::reg_scratch_0,
64-
arch::ptr_resolve(env.as, arch::fp, offset, arch::reg_scratch_1));
58+
env.as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
6559
env.as->blr(arch::reg_scratch_br);
6660
env.as->bind(after_call);
6761
}

cinderx/Jit/frame.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,21 @@ uintptr_t getIP(_PyInterpreterFrame* frame, int frame_size) {
118118
memcpy(&ip, saved_ip, kPointerSize);
119119
return ip;
120120
#elif defined(__aarch64__)
121-
// On aarch64, the JIT prologue does:
122-
// stp x29, x30, [sp, #-16]! ; saves FP and LR
123-
// mov x29, sp ; sets up frame pointer
124-
// In lightweight frame mode, frame_base == x29 (verified from
125-
// frameHeaderSize() computation). The saved LR (return address)
126-
// is at [frame_base + 8].
127-
//
128-
// We read it directly rather than walking the FP chain, because
129-
// intermediate C frames may be compiled without frame pointers
130-
// (e.g. pytest/pluggy extensions), breaking the chain walk.
121+
// On aarch64, the JIT saves the return address to [SP, #8] before each
122+
// BL/BLR. This slot is at frame_base - frame_size - kPointerSize
123+
// (= SP + 8, since SP = FP - stack_frame_size and frame_size =
124+
// stack_frame_size - kStackAlign). If no call has happened yet (slot
125+
// is 0 from prologue zero-init), fall back to the saved LR from the
126+
// STP at [frame_base + kPointerSize].
131127
uintptr_t ip;
132128
auto saved_ip =
133-
reinterpret_cast<uintptr_t*>(frame_base + kPointerSize);
129+
reinterpret_cast<uintptr_t*>(frame_base - frame_size - kPointerSize);
134130
memcpy(&ip, saved_ip, kPointerSize);
131+
if (ip == 0) {
132+
auto saved_lr =
133+
reinterpret_cast<uintptr_t*>(frame_base + kPointerSize);
134+
memcpy(&ip, saved_lr, kPointerSize);
135+
}
135136
return ip;
136137
#else
137138
// Unsupported architecture.
@@ -444,6 +445,7 @@ _PyInterpreterFrame* convertInterpreterFrameFromStackToSlab(
444445
}
445446

446447
jitFramePopulateFrame(frame);
448+
updatePrevInstr(frame);
447449
jitFrameRemoveReifier(frame);
448450

449451
memcpy(new_frame, frame, code->co_framesize * sizeof(PyObject*));

0 commit comments

Comments
 (0)