Skip to content

Commit 5aab98b

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Don't generate trampoline for deferred static compile failures
Summary: At some point this was necessary - we didn't *always* call the native entrypoint w/ the first argument. But that time's long gone. Instead of generate a trampoline at the end of the function let's just go to the de-opt trampoline and have it grab the right function. Reviewed By: alexmalyshev Differential Revision: D96419335 fbshipit-source-id: c75e733b8fa9f689fd42c05985f72da60e8d4a08
1 parent a9dcc50 commit 5aab98b

4 files changed

Lines changed: 11 additions & 40 deletions

File tree

cinderx/Jit/codegen/environ.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ struct Environ {
7474
explicit IndirectInfo(void** indirect_ptr) : indirect(indirect_ptr) {}
7575

7676
void** indirect;
77-
asmjit::Label trampoline{0};
7877
};
7978
UnorderedMap<PyFunctionObject*, IndirectInfo> function_indirections;
8079

cinderx/Jit/codegen/gen_asm.cpp

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -921,9 +921,7 @@ void* generateFailedDeferredCompileTrampoline() {
921921

922922
annot.add("saveRegisters", &a, annot_cursor);
923923

924-
// r10 contains the function object from our stub
925-
a.mov(x86::rdi, x86::r10);
926-
a.mov(x86::rsi, x86::rsp);
924+
a.mov(x86::rdi, x86::rsp);
927925
a.call(reinterpret_cast<uint64_t>(JITRT_FailedDeferredCompileShim));
928926
a.leave();
929927
a.ret();
@@ -941,9 +939,7 @@ void* generateFailedDeferredCompileTrampoline() {
941939

942940
annot.add("saveRegisters", &a, annot_cursor);
943941

944-
// x10 contains the function object from our stub
945-
a.mov(a64::x0, a64::x10);
946-
a.mov(a64::x1, a64::sp);
942+
a.mov(a64::x0, a64::sp);
947943
a.mov(arch::reg_scratch_br, JITRT_FailedDeferredCompileShim);
948944
a.blr(arch::reg_scratch_br);
949945
a.mov(a64::sp, arch::fp);
@@ -2076,17 +2072,6 @@ void NativeGenerator::generateEpilogue(BaseNode* epilogue_cursor) {
20762072
"Epilogue (restore regs; pop native frame; error exit)",
20772073
epilogue_error_cursor);
20782074
env_.addAnnotation("Epilogue", epilogue_cursor);
2079-
if (env_.function_indirections.size()) {
2080-
auto jit_helpers = as_->cursor();
2081-
for (auto& x : env_.function_indirections) {
2082-
Label trampoline = as_->newLabel();
2083-
as_->bind(trampoline);
2084-
as_->mov(x86::r10, reinterpret_cast<uint64_t>(x.first));
2085-
as_->jmp(reinterpret_cast<uint64_t>(failed_deferred_compile_trampoline_));
2086-
x.second.trampoline = trampoline;
2087-
}
2088-
env_.addAnnotation("JitHelpers", jit_helpers);
2089-
}
20902075
#elif defined(CINDER_AARCH64)
20912076
bool is_gen = GetFunction()->code->co_flags & kCoFlagsAnyGenerator;
20922077
if (is_gen) {
@@ -2172,18 +2157,6 @@ void NativeGenerator::generateEpilogue(BaseNode* epilogue_cursor) {
21722157
"Epilogue (restore regs; pop native frame; error exit)",
21732158
epilogue_error_cursor);
21742159
env_.addAnnotation("Epilogue", epilogue_cursor);
2175-
if (env_.function_indirections.size()) {
2176-
auto jit_helpers = as_->cursor();
2177-
for (auto& x : env_.function_indirections) {
2178-
Label trampoline = as_->newLabel();
2179-
as_->bind(trampoline);
2180-
as_->mov(a64::x10, reinterpret_cast<uint64_t>(x.first));
2181-
as_->mov(arch::reg_scratch_br, failed_deferred_compile_trampoline_);
2182-
as_->br(arch::reg_scratch_br);
2183-
x.second.trampoline = trampoline;
2184-
}
2185-
env_.addAnnotation("JitHelpers", jit_helpers);
2186-
}
21872160
#else
21882161
CINDER_UNSUPPORTED
21892162
#endif
@@ -2956,10 +2929,7 @@ void NativeGenerator::generateCode(CodeHolder& codeholder) {
29562929
{
29572930
ThreadedCompileSerialize guard;
29582931
for (auto& x : env_.function_indirections) {
2959-
Label trampoline = x.second.trampoline;
2960-
*x.second.indirect = reinterpret_cast<void*>(
2961-
codeholder.labelOffsetFromBase(trampoline) +
2962-
codeholder.baseAddress());
2932+
*x.second.indirect = failed_deferred_compile_trampoline_;
29632933
}
29642934
}
29652935

cinderx/Jit/jit_rt.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,11 +1769,12 @@ PyObject* JITRT_BuildString(
17691769
return _PyUnicode_JoinArray(empty, args, nargs);
17701770
}
17711771

1772-
JITRT_StaticCallReturn JITRT_FailedDeferredCompileShim(
1773-
PyFunctionObject* func,
1774-
PyObject** args) {
1772+
JITRT_StaticCallReturn JITRT_FailedDeferredCompileShim(PyObject** args) {
17751773
void* no_error = reinterpret_cast<void*>(1);
17761774

1775+
// The function object is always the first argument in the static calling
1776+
// convention.
1777+
PyFunctionObject* func = reinterpret_cast<PyFunctionObject*>(args[0]);
17771778
PyCodeObject* code = (PyCodeObject*)func->func_code;
17781779
int total_args = code->co_argcount;
17791780
if (code->co_flags & CO_VARARGS) {

cinderx/Jit/jit_rt.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,11 @@ PyObject* JITRT_UnpackExToTuple(
469469
* work in this function would have to be done anyway if we were initially
470470
* making a JIT static -> non-JIT static function anyway, so there is not too
471471
* much overhead.
472+
*
473+
* The function object is obtained from args[0] since in the static calling
474+
* convention the function is always the first argument.
472475
*/
473-
JITRT_StaticCallReturn JITRT_FailedDeferredCompileShim(
474-
PyFunctionObject* func,
475-
PyObject** args);
476+
JITRT_StaticCallReturn JITRT_FailedDeferredCompileShim(PyObject** args);
476477

477478
JITRT_StaticCallReturn JITRT_CallStaticallyWithPrimitiveSignature(
478479
PyFunctionObject* func,

0 commit comments

Comments
 (0)