Skip to content

Commit 6cb245d

Browse files
gh-143183: Link trace to side exits, rather than stop (GH-143268)
1 parent f37f57d commit 6cb245d

File tree

3 files changed

+46
-7
lines changed

3 files changed

+46
-7
lines changed

Lib/test/test_capi/test_opt.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ def iter_opnames(ex):
6060
def get_opnames(ex):
6161
return list(iter_opnames(ex))
6262

63+
def iter_ops(ex):
64+
for item in ex:
65+
yield item
66+
67+
def get_ops(ex):
68+
return list(iter_ops(ex))
69+
6370

6471
@requires_specialization
6572
@unittest.skipIf(Py_GIL_DISABLED, "optimizer not yet supported in free-threaded builds")
@@ -3003,14 +3010,25 @@ def f():
30033010
# Outer loop warms up later, linking to the inner one.
30043011
# Therefore, we have at least two executors.
30053012
self.assertGreaterEqual(len(all_executors), 2)
3013+
executor_ids = [id(e) for e in all_executors]
30063014
for executor in all_executors:
3007-
opnames = list(get_opnames(executor))
3015+
ops = get_ops(executor)
30083016
# Assert all executors first terminator ends in
30093017
# _EXIT_TRACE or _JUMP_TO_TOP, not _DEOPT
3010-
for idx, op in enumerate(opnames):
3011-
if op == "_EXIT_TRACE" or op == "_JUMP_TO_TOP":
3018+
for idx, op in enumerate(ops):
3019+
opname = op[0]
3020+
if opname == "_EXIT_TRACE":
3021+
# As this is a link outer executor to inner
3022+
# executor problem, all executors exits should point to
3023+
# another valid executor. In this case, none of them
3024+
# should be the cold executor.
3025+
exit = op[3]
3026+
link_to = _testinternalcapi.get_exit_executor(exit)
3027+
self.assertIn(id(link_to), executor_ids)
3028+
break
3029+
elif opname == "_JUMP_TO_TOP":
30123030
break
3013-
elif op == "_DEOPT":
3031+
elif opname == "_DEOPT":
30143032
self.fail(f"_DEOPT encountered first at executor"
30153033
f" {executor} at offset {idx} rather"
30163034
f" than expected _EXIT_TRACE")

Modules/_testinternalcapi.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,22 @@ invalidate_executors(PyObject *self, PyObject *obj)
12451245
Py_RETURN_NONE;
12461246
}
12471247

1248+
static PyObject *
1249+
get_exit_executor(PyObject *self, PyObject *arg)
1250+
{
1251+
if (!PyLong_CheckExact(arg)) {
1252+
PyErr_SetString(PyExc_TypeError, "argument must be an ID to an _PyExitData");
1253+
return NULL;
1254+
}
1255+
uint64_t ptr;
1256+
if (PyLong_AsUInt64(arg, &ptr) < 0) {
1257+
// Error set by PyLong API
1258+
return NULL;
1259+
}
1260+
_PyExitData *exit = (_PyExitData *)ptr;
1261+
return Py_NewRef(exit->executor);
1262+
}
1263+
12481264
#endif
12491265

12501266
static int _pending_callback(void *arg)
@@ -2546,6 +2562,7 @@ static PyMethodDef module_functions[] = {
25462562
#ifdef _Py_TIER2
25472563
{"add_executor_dependency", add_executor_dependency, METH_VARARGS, NULL},
25482564
{"invalidate_executors", invalidate_executors, METH_O, NULL},
2565+
{"get_exit_executor", get_exit_executor, METH_O, NULL},
25492566
#endif
25502567
{"pending_threadfunc", _PyCFunction_CAST(pending_threadfunc),
25512568
METH_VARARGS | METH_KEYWORDS},

Python/optimizer.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,6 @@ _PyJit_translate_single_bytecode_to_trace(
625625
int trace_length = _tstate->jit_tracer_state.prev_state.code_curr_size;
626626
_PyUOpInstruction *trace = _tstate->jit_tracer_state.code_buffer;
627627
int max_length = _tstate->jit_tracer_state.prev_state.code_max_size;
628-
int exit_op = stop_tracing_opcode == 0 ? _EXIT_TRACE : stop_tracing_opcode;
629628

630629
_Py_CODEUNIT *this_instr = _tstate->jit_tracer_state.prev_state.instr;
631630
_Py_CODEUNIT *target_instr = this_instr;
@@ -691,13 +690,18 @@ _PyJit_translate_single_bytecode_to_trace(
691690
goto full;
692691
}
693692

694-
if (stop_tracing_opcode != 0) {
693+
if (stop_tracing_opcode == _DEOPT) {
695694
// gh-143183: It's important we rewind to the last known proper target.
696695
// The current target might be garbage as stop tracing usually indicates
697696
// we are in something that we can't trace.
698697
DPRINTF(2, "Told to stop tracing\n");
699698
goto unsupported;
700699
}
700+
else if (stop_tracing_opcode != 0) {
701+
assert(stop_tracing_opcode == _EXIT_TRACE);
702+
ADD_TO_TRACE(stop_tracing_opcode, 0, 0, target);
703+
goto done;
704+
}
701705

702706
DPRINTF(2, "%p %d: %s(%d) %d %d\n", old_code, target, _PyOpcode_OpName[opcode], oparg, needs_guard_ip, old_stack_level);
703707

@@ -733,7 +737,7 @@ _PyJit_translate_single_bytecode_to_trace(
733737
int32_t old_target = (int32_t)uop_get_target(curr);
734738
curr++;
735739
trace_length++;
736-
curr->opcode = exit_op;
740+
curr->opcode = _DEOPT;
737741
curr->format = UOP_FORMAT_TARGET;
738742
curr->target = old_target;
739743
}

0 commit comments

Comments
 (0)