Skip to content

Commit 322d20f

Browse files
committed
Phase 4b: FP-relative saved-IP slot (eliminates kVectorcallArgsOffset overlap)
Store the saved-IP at [FP + saved_ip_fp_offset] instead of [SP, #8] to eliminate the theoretical overlap between the saved-IP slot and vectorcall args[0] at kVectorcallArgsOffset (also [SP+8]). Key change: pre-compute saved_ip_fp_offset in getVectorcallEntry after register allocation but before generateCode, because generateAssemblyBody (which emits ADR+STR for saved-IP) runs before computeFrameInfo. A JIT_CHECK in computeFrameInfo verifies the pre-computed value matches. Files changed: environ.h - add saved_ip_fp_offset field to Environ gen_asm.cpp - early offset computation + JIT_CHECK assertion gen_asm_utils.cpp - emitCall uses FP-relative ptr_resolve autogen.cpp - translateCall uses FP-relative ptr_resolve code_runtime.h/cpp - savedIpFpOffset accessor for CodeRuntime frame.cpp - getIP reads from FP + savedIpFpOffset frame_shadow.cpp - shadow frame getIP uses same offset Test results (15/15 CPU, PYTHONJITCOMPILATIONTHRESHOLD=1000): 11 PASS, 4 FAIL (all pre-existing, verified with CINDERJIT_DISABLE=1)
1 parent 6812609 commit 322d20f

8 files changed

Lines changed: 98 additions & 23 deletions

File tree

cinderx/Jit/code_runtime.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ void CodeRuntime::setFrameSize(int size) {
116116
frame_size_ = size;
117117
}
118118

119+
#if defined(__aarch64__)
120+
int CodeRuntime::savedIpFpOffset() const {
121+
return saved_ip_fp_offset_;
122+
}
123+
124+
void CodeRuntime::setSavedIpFpOffset(int offset) {
125+
saved_ip_fp_offset_ = offset;
126+
}
127+
#endif
128+
119129
DebugInfo* CodeRuntime::debugInfo() {
120130
return &debug_info_;
121131
}

cinderx/Jit/code_runtime.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ class alignas(16) CodeRuntime {
127127
// Get and set the total size of a stack frame for this compiled code object.
128128
int frameSize() const;
129129
void setFrameSize(int size);
130+
#if defined(__aarch64__)
131+
int savedIpFpOffset() const;
132+
void setSavedIpFpOffset(int offset);
133+
#endif
130134

131135
DebugInfo* debugInfo();
132136

@@ -162,6 +166,9 @@ class alignas(16) CodeRuntime {
162166
#endif
163167

164168
int frame_size_{-1};
169+
#if defined(__aarch64__)
170+
int saved_ip_fp_offset_{0};
171+
#endif
165172
DebugInfo debug_info_;
166173
};
167174

cinderx/Jit/codegen/autogen.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,8 +1937,6 @@ 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).
19421940
asmjit::Label after_call = as->newLabel();
19431941

19441942
if (input->isReg()) {
@@ -1947,20 +1945,29 @@ void translateCall(Environ* env, const Instruction* instr) {
19471945
as->mov(arch::reg_scratch_br, target);
19481946
}
19491947
as->adr(arch::reg_scratch_0, after_call);
1950-
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
1948+
as->str(
1949+
arch::reg_scratch_0,
1950+
arch::ptr_resolve(
1951+
as, arch::fp, env->saved_ip_fp_offset, arch::reg_scratch_1));
19511952
as->blr(arch::reg_scratch_br);
19521953
} else if (input->isImm()) {
19531954
as->mov(arch::reg_scratch_br, input->getConstant());
19541955
as->adr(arch::reg_scratch_0, after_call);
1955-
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
1956+
as->str(
1957+
arch::reg_scratch_0,
1958+
arch::ptr_resolve(
1959+
as, arch::fp, env->saved_ip_fp_offset, arch::reg_scratch_1));
19561960
as->blr(arch::reg_scratch_br);
19571961
} else if (input->isStack()) {
19581962
auto loc = input->getStackSlot().loc;
19591963
as->ldr(
19601964
arch::reg_scratch_br,
19611965
arch::ptr_resolve(as, arch::fp, loc, arch::reg_scratch_0));
19621966
as->adr(arch::reg_scratch_0, after_call);
1963-
as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
1967+
as->str(
1968+
arch::reg_scratch_0,
1969+
arch::ptr_resolve(
1970+
as, arch::fp, env->saved_ip_fp_offset, arch::reg_scratch_1));
19641971
as->blr(arch::reg_scratch_br);
19651972
} else {
19661973
JIT_ABORT("Unsupported operand type for Call: {}", input->type());

cinderx/Jit/codegen/environ.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ struct Environ {
2828
// functions.
2929
int stack_frame_size{-1};
3030

31+
#if defined(CINDER_AARCH64)
32+
// FP-relative offset of the saved-IP slot. Set by computeFrameInfo.
33+
int saved_ip_fp_offset{0};
34+
#endif
35+
3136
// A subset of stack_frame_size: only the shadow frames and spilled values.
3237
int shadow_frames_and_spill_size{0};
3338

cinderx/Jit/codegen/gen_asm.cpp

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,24 @@ void* NativeGenerator::getVectorcallEntry() {
12971297
*lir_func);
12981298
}
12991299

1300+
1301+
#if defined(CINDER_AARCH64)
1302+
// Pre-compute saved_ip_fp_offset before generateCode, because
1303+
// emitCall/translateCall in generateAssemblyBody need this offset to
1304+
// emit the ADR+STR saved-IP stores. generateAssemblyBody runs before
1305+
// computeFrameInfo, so the offset must be computed here.
1306+
{
1307+
int hdr = std::max(env_.shadow_frames_and_spill_size, kPointerSize)
1308+
+ kStackAlign;
1309+
auto saved_regs = env_.changed_regs & CALLEE_SAVE_REGS;
1310+
int saved_regs_sz = ((saved_regs.count() + 1) / 2) * kStackAlign;
1311+
if ((hdr + saved_regs_sz + env_.max_arg_buffer_size) % kStackAlign) {
1312+
hdr += kPointerSize;
1313+
}
1314+
env_.saved_ip_fp_offset = -(hdr - 8);
1315+
}
1316+
#endif
1317+
13001318
lir_func_ = std::move(lir_func);
13011319

13021320
try {
@@ -1329,7 +1347,8 @@ void* NativeGenerator::getVectorcallEntry() {
13291347
// The physical frame includes kStackAlign extra bytes for the saved-IP
13301348
// slot, but getIP() uses frame_base - frame_size - kPointerSize which
13311349
// must point to SP + 8 (where the ADR+STR saves the return address).
1332-
env_.code_rt->setFrameSize(env_.stack_frame_size - kStackAlign);
1350+
env_.code_rt->setFrameSize(env_.stack_frame_size);
1351+
env_.code_rt->setSavedIpFpOffset(env_.saved_ip_fp_offset);
13331352
#else
13341353
env_.code_rt->setFrameSize(env_.stack_frame_size);
13351354
#endif
@@ -1437,6 +1456,13 @@ NativeGenerator::FrameInfo NativeGenerator::computeFrameInfo() {
14371456
env_.last_callee_saved_reg_off =
14381457
info.header_and_spill_size + info.saved_regs_size();
14391458
env_.stack_frame_size = info.size();
1459+
#if defined(CINDER_AARCH64)
1460+
JIT_CHECK(
1461+
env_.saved_ip_fp_offset == -(info.header_and_spill_size - 8),
1462+
"saved_ip_fp_offset mismatch: pre-computed={}, computed={}",
1463+
env_.saved_ip_fp_offset,
1464+
-(info.header_and_spill_size - 8));
1465+
#endif
14401466
return info;
14411467
}
14421468

@@ -1457,9 +1483,6 @@ int NativeGenerator::allocateHeaderAndSpillSpace(const FrameInfo& frame_info) {
14571483
as_->sub(a64::sp, a64::sp, arch::reg_scratch_0);
14581484
}
14591485
}
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));
14631486

14641487
// There is a difference here from x86-64, because the aarch64 stack cannot be
14651488
// misaligned. Here we are returning the amount of space that we have added to
@@ -1505,6 +1528,12 @@ void NativeGenerator::saveCallerRegisters(
15051528
as_->sub(a64::sp, a64::sp, arch::reg_scratch_0);
15061529
}
15071530
}
1531+
// Zero-init the saved-IP slot at FP + saved_ip_fp_offset.
1532+
// getIP() falls back to [FP+8] (saved LR) when this slot reads 0.
1533+
as_->str(
1534+
a64::xzr,
1535+
arch::ptr_resolve(
1536+
as_, arch::fp, env_.saved_ip_fp_offset, arch::reg_scratch_0));
15081537
#else
15091538
CINDER_UNSUPPORTED
15101539
#endif

cinderx/Jit/codegen/gen_asm_utils.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ void emitCall(
2626
#if defined(CINDER_X86_64)
2727
env.as->call(label);
2828
#elif defined(CINDER_AARCH64)
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.
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.
3232
{
3333
asmjit::Label after_call = env.as->newLabel();
3434
env.as->adr(arch::reg_scratch_0, after_call);
35-
env.as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
35+
env.as->str(
36+
arch::reg_scratch_0,
37+
arch::ptr_resolve(
38+
env.as, arch::fp, env.saved_ip_fp_offset, arch::reg_scratch_1));
3639
env.as->bl(label);
3740
env.as->bind(after_call);
3841
}
@@ -51,11 +54,14 @@ void emitCall(Environ& env, uint64_t func, const jit::lir::Instruction* instr) {
5154
// https://github.com/asmjit/asmjit/issues/499, but as of writing is not yet
5255
// available.
5356
env.as->mov(arch::reg_scratch_br, func);
54-
// Save return address to [SP, #8] before BLR.
57+
// Save return address to stack before blr, matching x86 call semantics.
5558
{
5659
asmjit::Label after_call = env.as->newLabel();
5760
env.as->adr(arch::reg_scratch_0, after_call);
58-
env.as->str(arch::reg_scratch_0, asmjit::arm::Mem(asmjit::a64::sp, 8));
61+
env.as->str(
62+
arch::reg_scratch_0,
63+
arch::ptr_resolve(
64+
env.as, arch::fp, env.saved_ip_fp_offset, arch::reg_scratch_1));
5965
env.as->blr(arch::reg_scratch_br);
6066
env.as->bind(after_call);
6167
}

cinderx/Jit/frame.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,13 @@ 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 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].
121+
// On aarch64, frame_size is actually the FP-relative offset of the
122+
// saved-IP slot (savedIpFpOffset, a negative value). The slot is at
123+
// frame_base + offset. If no call has happened yet (slot is 0 from
124+
// prologue zero-init), fall back to saved LR at [frame_base + 8].
127125
uintptr_t ip;
128126
auto saved_ip =
129-
reinterpret_cast<uintptr_t*>(frame_base - frame_size - kPointerSize);
127+
reinterpret_cast<uintptr_t*>(frame_base + frame_size);
130128
memcpy(&ip, saved_ip, kPointerSize);
131129
if (ip == 0) {
132130
auto saved_lr =
@@ -198,7 +196,11 @@ UnitState getUnitState(_PyInterpreterFrame* frame) {
198196
_PyInterpreterFrame* non_inlined_sf = unit_frames[0];
199197
CodeRuntime* code_rt = getCodeRuntime(non_inlined_sf);
200198
JIT_CHECK(code_rt != nullptr, "failed to find code runtime");
199+
#if defined(__aarch64__)
200+
uintptr_t ip = getIP(non_inlined_sf, code_rt->savedIpFpOffset());
201+
#else
201202
uintptr_t ip = getIP(non_inlined_sf, code_rt->frameSize());
203+
#endif
202204
std::optional<UnitCallStack> locs =
203205
code_rt->debugInfo()->getUnitCallStack(ip);
204206
if (locs.has_value()) {

cinderx/Jit/frame_shadow.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,15 @@ uintptr_t getIP(_PyShadowFrame* shadow_frame, int frame_size) {
130130
} else {
131131
frame_base = getFrameBaseFromOnStackShadowFrame(shadow_frame);
132132
}
133-
// Read the saved IP from the stack
133+
// Read the saved IP from the stack.
134134
uintptr_t ip;
135+
#if defined(__aarch64__)
136+
auto saved_ip =
137+
reinterpret_cast<uintptr_t*>(frame_base + frame_size);
138+
#else
135139
auto saved_ip =
136140
reinterpret_cast<uintptr_t*>(frame_base - frame_size - kPointerSize);
141+
#endif
137142
memcpy(&ip, saved_ip, kPointerSize);
138143
return ip;
139144
}
@@ -381,7 +386,11 @@ UnitState getUnitState(_PyShadowFrame* shadow_frame) {
381386
unit_state.reserve(unit_frames.size());
382387
_PyShadowFrame* non_inlined_sf = unit_frames[0];
383388
CodeRuntime* code_rt = getCodeRuntime(non_inlined_sf);
389+
#if defined(__aarch64__)
390+
uintptr_t ip = getIP(non_inlined_sf, code_rt->savedIpFpOffset());
391+
#else
384392
uintptr_t ip = getIP(non_inlined_sf, code_rt->frameSize());
393+
#endif
385394
std::optional<UnitCallStack> locs =
386395
code_rt->debugInfo()->getUnitCallStack(ip);
387396
if (locs.has_value()) {

0 commit comments

Comments
 (0)