Skip to content

Commit c047204

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Flow thread state in and frame out
Summary: Currently for allocating a generator frame we use `PyThreadState_GET` to acquire the thread state in our runtime helper and then return it to the JITed function and then we pull the current frame from the thread state. But for any non-BOLTed build we're going to be able to acquire the thread state faster in JITed code through our runtime resolved address. Freeing up the return value also let's us get rid of the dependent loads to get the frame object. Overall that's less instructions as well. We end up saving 3 instructions but 1 of those is due to an inefficiency in `adr` and the other is due to a register allocator change. We could just pass 5 arguments into this function but that'll complicate the call on Windows so we shuffle the resume entry point into the code runtime which we're already reading anyway. Reviewed By: alexmalyshev Differential Revision: D114815510 fbshipit-source-id: 309cb32eb8e8d95a3e48e86869b7b4c158d81fb8
1 parent 6b877db commit c047204

6 files changed

Lines changed: 58 additions & 17 deletions

File tree

cinderx/Jit/code_runtime.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,14 @@ void CodeRuntime::setSpillWords(uint32_t words) {
130130
spill_words_ = words;
131131
}
132132

133+
GenResumeFunc CodeRuntime::genResumeEntry() const {
134+
return gen_resume_entry_;
135+
}
136+
137+
void CodeRuntime::setGenResumeEntry(GenResumeFunc resume_entry) {
138+
gen_resume_entry_ = resume_entry;
139+
}
140+
133141
DebugInfo* CodeRuntime::debugInfo() {
134142
return &debug_info_;
135143
}

cinderx/Jit/code_runtime.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#pragma once
44

55
#include "cinderx/Common/ref.h"
6+
#include "cinderx/Common/util.h"
67
#include "cinderx/Jit/debug_info.h"
78
#include "cinderx/Jit/deopt.h"
89
#include "cinderx/Jit/threaded_compile.h"
@@ -94,6 +95,12 @@ class alignas(16) CodeRuntime {
9495
uint32_t spillWords() const;
9596
void setSpillWords(uint32_t words);
9697

98+
// Get and set the address a generator resumes execution at. Only meaningful
99+
// for generators, and only resolvable once code generation has bound the
100+
// resume entry label to an address.
101+
GenResumeFunc genResumeEntry() const;
102+
void setGenResumeEntry(GenResumeFunc resume_entry);
103+
97104
DebugInfo* debugInfo();
98105

99106
// Allocate a jump table for static type check dispatch.
@@ -154,6 +161,8 @@ class alignas(16) CodeRuntime {
154161
// Set after CompiledFunction::create() in makeCompiledFunction().
155162
BorrowedRef<CompiledFunction> compiled_function_;
156163

164+
GenResumeFunc gen_resume_entry_{nullptr};
165+
157166
int frame_size_{-1};
158167
uint32_t spill_words_{0};
159168
DebugInfo debug_info_;

cinderx/Jit/codegen/gen_asm.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,6 +1431,17 @@ void NativeGenerator::generateCode(
14311431
codeholder.baseAddress());
14321432
}
14331433

1434+
// allocateAndLinkGenAndInterpreterFrame reads the resume entry out of the
1435+
// CodeRuntime rather than taking it as an argument, which keeps that call
1436+
// down to four arguments.
1437+
if (getFunction()->code->co_flags & kCoFlagsAnyGenerator) {
1438+
auto resume_entry = static_cast<uintptr_t>(
1439+
codeholder.baseAddress() +
1440+
codeholder.labelOffsetFromBase(env_.gen_resume_entry_label));
1441+
env_.code_rt->setGenResumeEntry(
1442+
reinterpret_cast<GenResumeFunc>(resume_entry));
1443+
}
1444+
14341445
// Resolve the static type check jump table entries now that block labels
14351446
// have been bound to code addresses.
14361447
for (auto& [index, block] : env_.static_typecheck_jt_entries) {

cinderx/Jit/jit_rt.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -716,10 +716,11 @@ void initFrameCellVars(
716716
#endif
717717
}
718718

719-
std::pair<PyThreadState*, GenDataFooter*> allocateAndLinkGenAndInterpreterFrame(
719+
std::pair<_PyInterpreterFrame*, GenDataFooter*>
720+
allocateAndLinkGenAndInterpreterFrame(
721+
PyThreadState* tstate,
720722
PyFunctionObject* func,
721723
CodeRuntime* code_rt,
722-
GenResumeFunc resume_func,
723724
uint64_t original_frame_pointer) {
724725
JIT_DCHECK(
725726
PyCode_Check(func->func_code),
@@ -729,7 +730,6 @@ std::pair<PyThreadState*, GenDataFooter*> allocateAndLinkGenAndInterpreterFrame(
729730
JIT_DCHECK(co == code_rt->code(), "Code object mismatch");
730731

731732
uint64_t spill_words = code_rt->spillWords();
732-
PyThreadState* tstate = PyThreadState_GET();
733733
JIT_DCHECK(tstate != nullptr, "thread state cannot be null");
734734
auto [gen, gen_size] = cinderx::getModuleState()->jit_gen_free_list->allocate(
735735
co, spill_words * sizeof(uint64_t) + sizeof(GenDataFooter));
@@ -777,6 +777,11 @@ std::pair<PyThreadState*, GenDataFooter*> allocateAndLinkGenAndInterpreterFrame(
777777
init_and_link_interpreter_frame(
778778
func, co, tstate, FRAME_OWNED_BY_GENERATOR, frame, code_rt);
779779

780+
GenResumeFunc resume_func = code_rt->genResumeEntry();
781+
JIT_DCHECK(
782+
resume_func != nullptr,
783+
"CodeRuntime has no resume entry: {}",
784+
PyUnicode_AsUTF8(co->co_qualname));
780785
footer->resumeEntry = resume_func;
781786
footer->yieldPoint = nullptr;
782787
footer->gen = static_cast<PyGenObject*>(gen);
@@ -812,7 +817,7 @@ std::pair<PyThreadState*, GenDataFooter*> allocateAndLinkGenAndInterpreterFrame(
812817

813818
PyObject_GC_Track(gen);
814819

815-
return {tstate, footer};
820+
return {frame, footer};
816821
}
817822

818823
std::pair<JitGenObject*, GenDataFooter*> unlinkGenFrameAndReturnGenDataFooter(

cinderx/Jit/jit_rt.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
#include "cinderx/python.h"
66

7+
#include "internal/pycore_frame.h"
8+
9+
#if PY_VERSION_HEX >= 0x030E0000
10+
#include "internal/pycore_interpframe_structs.h"
11+
#endif
12+
713
#include "cinderx/Common/util.h"
814
#include "cinderx/StaticPython/typed-args-info.h"
915

@@ -36,10 +42,14 @@ PyThreadState* allocateAndLinkInterpreterFrame_Debug(
3642

3743
PyThreadState* allocateAndLinkInterpreterFrame_Release(PyFunctionObject* func);
3844

39-
std::pair<PyThreadState*, GenDataFooter*> allocateAndLinkGenAndInterpreterFrame(
45+
// Allocate a generator and its interpreter frame, and link the frame into
46+
// tstate. Returns the newly linked interpreter frame and the generator's
47+
// GenDataFooter.
48+
std::pair<_PyInterpreterFrame*, GenDataFooter*>
49+
allocateAndLinkGenAndInterpreterFrame(
50+
PyThreadState* tstate,
4051
PyFunctionObject* func,
4152
CodeRuntime* code_rt,
42-
GenResumeFunc resume_entry,
4353
uint64_t original_frame_pointer);
4454

4555
void initFrameCellVars(

cinderx/Jit/lir/generator.cpp

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5494,32 +5494,30 @@ void LIRGenerator::emitLoadFrame(BasicBlockBuilder& bbb) {
54945494
bool is_gen =
54955495
func_->code != nullptr && (func_->code->co_flags & kCoFlagsAnyGenerator);
54965496
if (is_gen) {
5497-
// Load the resume entry label address.
5497+
bbb.annotateNext("Allocate generator + interpreter frame: load tstate");
5498+
env_->asm_tstate =
5499+
bbb.appendInstr(OutVReg{}, Instruction::kLoadThreadState);
5500+
// spill_words and the resume entry address are read from CodeRuntime by
5501+
// the runtime function, so we don't need to pass them explicitly.
54985502
bbb.annotateNext("Allocate generator + interpreter frame");
5499-
Instruction* resume_label = bbb.appendInstr(
5500-
OutVReg{DataType::k64bit},
5501-
Instruction::kLea,
5502-
AsmLbl{env_->gen_resume_entry_label});
5503-
// spill_words is read from CodeRuntime by the runtime function,
5504-
// so we don't need to pass it explicitly.
5503+
Instruction* frame;
55055504
Instruction* footer;
55065505
appendCall2RetValues(
55075506
bbb,
5508-
env_->asm_tstate,
5507+
frame,
55095508
footer,
55105509
rt::allocateAndLinkGenAndInterpreterFrame,
5510+
env_->asm_tstate,
55115511
env_->asm_func,
55125512
Imm{reinterpret_cast<uint64_t>(env_->code_rt)},
5513-
VReg{resume_label},
55145513
PhyReg{codegen::arch::reg_frame_pointer_loc});
55155514
// Swap RBP to point at the generator data so spills go there.
55165515
bbb.annotateNext("Set frame pointer to GenDataFooter");
55175516
bbb.appendInstr(
55185517
OutPhyReg{codegen::arch::reg_frame_pointer_loc},
55195518
Instruction::kMove,
55205519
VReg{footer});
5521-
bbb.annotateNext("Load current interpreter frame");
5522-
env_->asm_interpreter_frame = makeCurrentFrameAccessor(bbb).load();
5520+
env_->asm_interpreter_frame = frame;
55235521
#if (defined(CINDER_AARCH64) || defined(Py_GIL_DISABLED)) && \
55245522
defined(ENABLE_LIGHTWEIGHT_FRAMES)
55255523
// Now that FP points at the heap-allocated GenDataFooter, compute the

0 commit comments

Comments
 (0)