8000 GH-109369: Exit tier 2 if executor is invalid (GH-111657) · aisk/cpython@93cd550 · GitHub
[go: up one dir, main page]
Skip to content

Commit 93cd550

Browse files
markshannonaisk
markshannon
authored and
aisk
committed
pythonGH-109369: Exit tier 2 if executor is invalid (pythonGH-111657)
1 parent 3706873 commit 93cd550

File tree

11 files changed

+348
-230
lines changed

11 files changed

+348
-230
lines changed

‎Include/internal/pycore_opcode_metadata.h

Lines changed: 233 additions & 224 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_monitoring.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,3 +1790,28 @@ def test_func(x):
17901790
test_func(1000)
17911791
sys.monitoring.set_local_events(TEST_TOOL, code, 0)
17921792
self.assertEqual(sys.monitoring.get_local_events(TEST_TOOL, code), 0)
1793+
1794+
class TestTier2Optimizer(CheckEvents):
1795+
1796+
def test_monitoring_already_opimized_loop(self):
1797+
def test_func(recorder):
1798+
set_events = sys.monitoring.set_events
1799+
line = E.LINE
1800+
i = 0
1801+
for i in range(551):
1802+
# Turn on events without branching once i reaches 500.
1803+
set_events(TEST_TOOL, line * int(i >= 500))
1804+
pass
1805+
pass
1806+
pass
1807+
1808+
self.assertEqual(sys.monitoring._all_events(), {})
1809+
events = []
1810+
recorder = LineRecorder(events)
1811+
sys.monitoring.register_callback(TEST_TOOL, E.LINE, recorder)
1812+
try:
1813+
test_func(recorder)
1814+
finally:
1815+
sys.monitoring.register_callback(TEST_TOOL, E.LINE, None)
1816+
sys.monitoring.set_events(TEST_TOOL, 0)
1817+
self.assertGreater(len(events), 250)

‎Misc/NEWS.d/next/Core and Builtins/2023-11-03-22-48-29.gh-issue-109369.ELYaxJ.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make sure that tier 2 traces are de-optimized if the code is instrumented

‎Python/abstract_interp_cases.c.h

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/bytecodes.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4021,6 +4021,11 @@ dummy_func(
40214021
memmove(&stack_pointer[-1 - oparg], &stack_pointer[-oparg], oparg * sizeof(stack_pointer[0]));
40224022
}
40234023

4024+
op(_CHECK_VALIDITY, (--)) {
4025+
TIER_TWO_ONLY
4026+
DEOPT_IF(!current_executor->base.vm_data.valid);
4027+
}
4028+
40244029

40254030
// END BYTECODES //
40264031

‎Python/ceval.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1057,7 +1057,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
10571057
deoptimize:
10581058
// On DEOPT_IF we just repeat the last instruction.
10591059
// This presumes nothing was popped from the stack (nor pushed).
1060-
DPRINTF(2, "DEOPT: [Opcode %d, operand %" PRIu64 "]\n", opcode, operand);
1060+
DPRINTF(2, "DEOPT: [Opcode %d, operand %" PRIu64 " @ %d]\n", opcode, operand, (int)(next_uop-current_executor->trace-1));
10611061
OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
10621062
UOP_STAT_INC(opcode, miss);
10631063
frame->return_offset = 0; // Dispatch to frame->instr_ptr

‎Python/executor_cases.c.h

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/optimizer.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,8 @@ translate_bytecode_to_trace(
472472
} \
473473
reserved = (n); // Keep ADD_TO_TRACE / ADD_TO_STUB honest
474474

475-
// Reserve space for main+stub uops, plus 2 for _SET_IP and _EXIT_TRACE
476-
#define RESERVE(main, stub) RESERVE_RAW((main) + (stub) + 2, uop_name(opcode))
475+
// Reserve space for main+stub uops, plus 3 for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
476+
#define RESERVE(main, stub) RESERVE_RAW((main) + (stub) + 3, uop_name(opcode))
477477

478478
// Trace stack operations (used by _PUSH_FRAME, _POP_FRAME)
479479
#define TRACE_STACK_PUSH() \
@@ -503,8 +503,9 @@ translate_bytecode_to_trace(
503503

504504
top: // Jump here after _PUSH_FRAME or likely branches
505505
for (;;) {
506-
RESERVE_RAW(2, "epilogue"); // Always need space for _SET_IP and _EXIT_TRACE
506+
RESERVE_RAW(3, "epilogue"); // Always need space for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
507507
ADD_TO_TRACE(_SET_IP, INSTR_IP(instr, code), 0);
508+
ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0);
508509

509510
uint32_t opcode = instr->op.code;
510511
uint32_t oparg = instr->op.arg;

‎Python/optimizer_analysis.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212
#include <stddef.h>
1313
#include "pycore_optimizer.h"
1414

15-
1615
static void
1716
remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
1817
{
1918
// Note that we don't enter stubs, those SET_IPs are needed.
2019
int last_set_ip = -1;
2120
bool need_ip = true;
21+
bool maybe_invalid = false;
2222
for (int pc = 0; pc < buffer_size; pc++) {
2323
int opcode = buffer[pc].opcode;
2424
if (opcode == _SET_IP) {
@@ -28,6 +28,16 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
2828
need_ip = false;
2929
last_set_ip = pc;
3030
}
31+
else if (opcode == _CHECK_VALIDITY) {
32+
if (maybe_invalid) {
33+
/* Exiting the trace requires that IP is correct */
34+
need_ip = true;
35+
maybe_invalid = false;
36+
}
37+
else {
38+
buffer[pc].opcode = NOP;
39+
}
40+
}
3141
else if (opcode == _JUMP_TO_TOP || opcode == _EXIT_TRACE) {
3242
break;
3343
}
@@ -36,6 +46,9 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
3646
if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG) || opcode == _PUSH_FRAME) {
3747
need_ip = true;
3848
}
49+
if (_PyOpcode_opcode_metadata[opcode].flags & HAS_ESCAPES_FLAG) {
50+
maybe_invalid = true;
51+
}
3952
}
4053
}
4154
}

‎Tools/cases_generator/analysis.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction:
390390
else:
391391
targets.append(self.macro_instrs[target_name])
392392
assert targets
393-
ignored_flags = {"HAS_EVAL_BREAK_FLAG", "HAS_DEOPT_FLAG", "HAS_ERROR_FLAG"}
393+
ignored_flags = {"HAS_EVAL_BREAK_FLAG", "HAS_DEOPT_FLAG", "HAS_ERROR_FLAG", "HAS_ESCAPES_FLAG"}
394394
assert len({t.instr_flags.bitmap(ignore=ignored_flags) for t in targets}) == 1
395395
return PseudoInstruction(pseudo.name, targets, targets[0].instr_flags)
396396

0 commit comments

Comments
 (0)
0