Skip to content

Commit f55129e

Browse files
fix recursive tracing
1 parent a274451 commit f55129e

File tree

4 files changed

+65
-67
lines changed

4 files changed

+65
-67
lines changed

Include/internal/pycore_interp_structs.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,6 @@ struct _is {
944944
struct callable_cache callable_cache;
945945
PyObject *common_consts[NUM_COMMON_CONSTANTS];
946946
// JIT tracing state
947-
bool jit_is_tracing;
948947
int jit_tracer_code_max_size;
949948
int jit_tracer_code_curr_size;
950949
_PyBloomFilter jit_tracer_dependencies;
@@ -955,7 +954,8 @@ struct _is {
955954
PyCodeObject *jit_tracer_initial_code; // Strong
956955
PyFunctionObject *jit_tracer_initial_func; // Strong
957956
struct _PyExitData *jit_tracer_previous_exit;
958-
bool jit_completed_loop;
957+
_PyInterpreterFrame *jit_tracer_current_frame;
958+
// End Jit tracing state
959959
bool jit;
960960
bool compiling;
961961
struct _PyUOpInstruction *jit_uop_buffer;

Python/ceval.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,12 +1049,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
10491049
return NULL;
10501050
}
10511051

1052-
// We came in already tracing, this means a recursive trace in the form of
1053-
// Python -> C -> Python happened. We want to abandon the outer trace then.
1054-
if (IS_JIT_TRACING()) {
1055-
_PyJIT_FinalizeTracing(tstate);
1056-
tstate->interp->jit_is_tracing = false;
1057-
}
10581052
/* Local "register" variables.
10591053
* These are cached values from the frame and code object. */
10601054
_Py_CODEUNIT *next_instr;
@@ -1201,14 +1195,27 @@ _PyTier2Interpreter(
12011195
uopcode = next_uop->opcode;
12021196
#ifdef Py_DEBUG
12031197
if (frame->lltrace >= 2) {
1204-
// if (next_uop->opcode != _YIELD_VALUE &&
1205-
// next_uop->opcode != _FOR_ITER_GEN_FRAME &&
1206-
// next_uop->opcode != _PUSH_FRAME &&
1207-
// next_uop->opcode != _PY_FRAME_KW &&
1208-
// next_uop->opcode != _SAVE_RETURN_OFFSET &&
1209-
// next_uop->opcode != _SAVE_RETURN_OFFSET) {
1210-
// dump_stack(frame, stack_pointer);
1211-
// }
1198+
if (next_uop->opcode != _YIELD_VALUE &&
1199+
next_uop->opcode != _FOR_ITER_GEN_FRAME &&
1200+
next_uop->opcode != _PUSH_FRAME &&
1201+
next_uop->opcode != _PY_FRAME_KW &&
1202+
next_uop->opcode != _SAVE_RETURN_OFFSET &&
1203+
next_uop->opcode != _SAVE_RETURN_OFFSET) {
1204+
if (next_uop->opcode != _START_EXECUTOR) {
1205+
if (next_uop->format == UOP_FORMAT_TARGET) {
1206+
_Py_CODEUNIT *aim = _PyFrame_GetBytecode(frame) + next_uop->target;
1207+
printf(" aim=[%s]\n", _PyOpcode_OpName[aim->op.code]);
1208+
}
1209+
else if (next_uop->format == UOP_FORMAT_JUMP) {
1210+
_PyUOpInstruction *aim_uop = current_executor->trace + next_uop->jump_target;
1211+
if (aim_uop->format == UOP_FORMAT_TARGET) {
1212+
_Py_CODEUNIT *aim = _PyFrame_GetBytecode(frame) + aim_uop->target;
1213+
printf(" aim=[%s]\n", _PyOpcode_OpName[aim->op.code]);
1214+
}
1215+
}
1216+
}
1217+
dump_stack(frame, stack_pointer);
1218+
}
12121219
if (next_uop->opcode == _START_EXECUTOR) {
12131220
printf("%4d uop: ", 0);
12141221
}

Python/ceval_macros.h

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -134,25 +134,14 @@
134134
RECORD_JUMP_TAKEN() \
135135
RECORD_TRACE_NO_DISPATCH() \
136136
assert(!IS_JIT_TRACING()); \
137-
RELOAD_TRACING(); \
138137
JUMP_TO_LABEL(label);
139138

140139
#if _Py_TAIL_CALL_INTERP || USE_COMPUTED_GOTOS
141-
# define IS_JIT_TRACING() (tstate->interp->jit_is_tracing)
142-
# define SET_TRACING() \
143-
DISPATCH_TABLE_VAR = TRACING_DISPATCH_TABLE;
140+
# define IS_JIT_TRACING() (DISPATCH_TABLE_VAR == TRACING_DISPATCH_TABLE)
144141
# define ENTER_TRACING() \
145-
SET_TRACING(); \
146-
tstate->interp->jit_is_tracing = true;
147-
# define UNSET_TRACING() \
148-
DISPATCH_TABLE_VAR = DISPATCH_TABLE;
142+
DISPATCH_TABLE_VAR = TRACING_DISPATCH_TABLE;
149143
# define LEAVE_TRACING() \
150-
assert(IS_JIT_TRACING()); \
151-
UNSET_TRACING(); \
152-
tstate->interp->jit_is_tracing = false;
153-
// This handles recursive tracing over C calls.
154-
# define RELOAD_TRACING() \
155-
DISPATCH_TABLE_VAR = tstate->interp->jit_is_tracing ? TRACING_DISPATCH_TABLE : DISPATCH_TABLE;
144+
DISPATCH_TABLE_VAR = DISPATCH_TABLE;
156145
# define BAIL_TRACING_NO_DISPATCH() \
157146
LEAVE_TRACING(); \
158147
_PyFrame_SetStackPointer(frame, stack_pointer); \
@@ -224,14 +213,12 @@ do { \
224213
} while (0)
225214

226215
#define TRACING_DISPATCH_INLINED(NEW_FRAME) \
227-
RELOAD_TRACING(); \
228216
RECORD_TRACE_NO_DISPATCH(); \
229217
DISPATCH_INLINED(NEW_FRAME);
230218

231219
#define TRACING_DISPATCH() \
232220
{ \
233221
assert(frame->stackpointer == NULL); \
234-
RELOAD_TRACING(); \
235222
RECORD_TRACE_NO_DISPATCH(); \
236223
NEXTOPARG(); \
237224
PRE_DISPATCH_GOTO(); \

Python/optimizer.c

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,8 @@ _PyJIT_translate_single_bytecode_to_trace(
562562
if (Py_IsNone((PyObject *)func)) {
563563
func = NULL;
564564
}
565-
int is_first_instr = tstate->interp->jit_tracer_initial_instr == this_instr ;
565+
566+
int is_first_instr = tstate->interp->jit_tracer_initial_instr == this_instr;
566567
bool progress_needed = (tstate->interp->jit_tracer_initial_chain_depth % MAX_CHAIN_DEPTH) == 0 && is_first_instr;;
567568
_PyBloomFilter *dependencies = &tstate->interp->jit_tracer_dependencies;
568569
_Py_BloomFilter_Add(dependencies, old_code);
@@ -583,7 +584,39 @@ _PyJIT_translate_single_bytecode_to_trace(
583584

584585
target = INSTR_IP(target_instr, old_code);
585586

586-
DPRINTF(2, "%d: %s(%d)\n", target, _PyOpcode_OpName[opcode], oparg);
587+
bool needs_guard_ip = _PyOpcode_NeedsGuardIp[opcode] &&
588+
!(opcode == FOR_ITER_RANGE || opcode == FOR_ITER_LIST || opcode == FOR_ITER_TUPLE) &&
589+
!(opcode == JUMP_BACKWARD_NO_INTERRUPT || opcode == JUMP_BACKWARD || opcode == JUMP_BACKWARD_JIT) &&
590+
!(opcode == POP_JUMP_IF_TRUE || opcode == POP_JUMP_IF_FALSE || opcode == POP_JUMP_IF_NONE || opcode == POP_JUMP_IF_NOT_NONE);
591+
592+
// Strange control-flow, unsupported opcode, etc.
593+
if (jump_taken ||
594+
// This happens when a recursive call happens that we can't trace. Such as Python -> C -> Python calls
595+
// If we haven't guarded the IP, then it's untraceable.
596+
(frame != tstate->interp->jit_tracer_current_frame && !needs_guard_ip) ||
597+
// TODO handle extended args.
598+
oparg > 255 || opcode == EXTENDED_ARG ||
599+
opcode == WITH_EXCEPT_START || opcode == RERAISE || opcode == CLEANUP_THROW || opcode == PUSH_EXC_INFO ||
600+
frame->owner >= FRAME_OWNED_BY_INTERPRETER ||
601+
// This can be supported, but requires a tracing shim frame.
602+
opcode == CALL_ALLOC_AND_ENTER_INIT) {
603+
unsupported:
604+
{
605+
// Rewind to previous instruction and replace with _EXIT_TRACE.
606+
_PyUOpInstruction *curr = &trace[trace_length-1];
607+
while (curr->opcode != _SET_IP && trace_length > 1) {
608+
trace_length--;
609+
curr = &trace[trace_length-1];
610+
}
611+
assert(curr->opcode == _SET_IP || trace_length == 1);
612+
curr->opcode = _EXIT_TRACE;
613+
goto done;
614+
}
615+
}
616+
617+
tstate->interp->jit_tracer_current_frame = frame;
618+
619+
DPRINTF(2, "%p %d: %s(%d)\n", old_code, target, _PyOpcode_OpName[opcode], oparg);
587620

588621
if (opcode == NOP) {
589622
return 1;
@@ -601,47 +634,18 @@ _PyJIT_translate_single_bytecode_to_trace(
601634
max_length -= 2;
602635

603636
if (opcode == ENTER_EXECUTOR) {
604-
ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0, target);
605-
ADD_TO_TRACE(_SET_IP, 0, (uintptr_t)target_instr, target);
606-
ADD_TO_TRACE(_EXIT_TRACE, 0, 0, target);
607637
goto full;
608638
}
609639

610-
bool needs_guard_ip = _PyOpcode_NeedsGuardIp[opcode] &&
611-
!(opcode == FOR_ITER_RANGE || opcode == FOR_ITER_LIST || opcode == FOR_ITER_TUPLE) &&
612-
!(opcode == JUMP_BACKWARD_NO_INTERRUPT || opcode == JUMP_BACKWARD || opcode == JUMP_BACKWARD_JIT) &&
613-
!(opcode == POP_JUMP_IF_TRUE || opcode == POP_JUMP_IF_FALSE || opcode == POP_JUMP_IF_NONE || opcode == POP_JUMP_IF_NOT_NONE);
614-
615-
assert(opcode != ENTER_EXECUTOR && opcode != EXTENDED_ARG);
616-
617640
const struct opcode_macro_expansion *expansion = &_PyOpcode_macro_expansion[opcode];
618641

619-
// Strange control-flow, unsupported opcode, etc.
620-
if (jump_taken ||
621-
// TODO handle extended args.
622-
oparg > 255 ||
623-
opcode == EXTENDED_ARG ||
624-
opcode == WITH_EXCEPT_START || opcode == RERAISE || opcode == CLEANUP_THROW || opcode == PUSH_EXC_INFO ||
625-
frame->owner >= FRAME_OWNED_BY_INTERPRETER ||
626-
// This can be supported, but requires a tracing shim frame.
627-
opcode == CALL_ALLOC_AND_ENTER_INIT) {
628-
unsupported:
629-
{
630-
// Rewind to previous instruction and replace with _EXIT_TRACE.
631-
_PyUOpInstruction *curr = &trace[trace_length-1];
632-
while (curr->opcode != _SET_IP && trace_length > 1) {
633-
trace_length--;
634-
curr = &trace[trace_length-1];
635-
}
636-
assert(curr->opcode == _SET_IP || trace_length == 1);
637-
curr->opcode = _EXIT_TRACE;
638-
goto done;
639-
}
640-
}
641642
RESERVE_RAW(expansion->nuops + needs_guard_ip + 3, "uop and various checks");
642643

643644
ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0, target);
644645

646+
assert(opcode != ENTER_EXECUTOR && opcode != EXTENDED_ARG);
647+
assert(!_PyErr_Occurred(tstate));
648+
645649
if (!OPCODE_HAS_NO_SAVE_IP(opcode)) {
646650
ADD_TO_TRACE(_SET_IP, 0, (uintptr_t)target_instr, target);
647651
}
@@ -851,9 +855,9 @@ _PyJIT_InitializeTracing(PyThreadState *tstate, _PyInterpreterFrame *frame, _Py_
851855
tstate->interp->jit_tracer_initial_func = (PyFunctionObject *)Py_NewRef(_PyFrame_GetFunction(frame));
852856
tstate->interp->jit_tracer_previous_exit = exit;
853857
memset(&tstate->interp->jit_tracer_dependencies.bits, 0, sizeof(tstate->interp->jit_tracer_dependencies.bits));
854-
tstate->interp->jit_completed_loop = false;
855858
tstate->interp->jit_tracer_initial_stack_depth = curr_stackdepth;
856859
tstate->interp->jit_tracer_initial_chain_depth = chain_depth;
860+
tstate->interp->jit_tracer_current_frame = frame;
857861
}
858862

859863
void

0 commit comments

Comments
 (0)