GH-109369: Exit tier 2 if executor is invalid

Lib/test/test_monitoring.py

@@ -1790,3 +1790,28 @@ def test_func(x):
  test_func(1000)
  sys.monitoring.set_local_events(TEST_TOOL, code, 0)
  self.assertEqual(sys.monitoring.get_local_events(TEST_TOOL, code), 0)
+
+class TestTier2Optimizer(CheckEvents):
+
+ def test_monitoring_already_opimized_loop(self):
+ def test_func(recorder):
+ set_events = sys.monitoring.set_events
+ line = E.LINE
+ i = 0
+ for i in range(551):
+ # Turn on events without branching once i reaches 500.
+ set_events(TEST_TOOL, line * int(i >= 500))
+ pass
+ pass
+ pass
+
+ self.assertEqual(sys.monitoring._all_events(),{})
+ events = []
+ recorder = LineRecorder(events)
+ sys.monitoring.register_callback(TEST_TOOL, E.LINE, recorder)
+ try:
+ test_func(recorder)
+ finally:
+ sys.monitoring.register_callback(TEST_TOOL, E.LINE, None)
+ sys.monitoring.set_events(TEST_TOOL, 0)
+ self.assertGreater(len(events), 250)

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

@@ -0,0 +1 @@
+Make sure that tier 2 traces are de-optimized if the code is instrumented

Python/bytecodes.c

@@ -4021,6 +4021,11 @@ dummy_func(
  memmove(&stack_pointer[-1 - oparg], &stack_pointer[-oparg], oparg * sizeof(stack_pointer[0]));
  }
 
+ op(_CHECK_VALIDITY, (--)){
+ TIER_TWO_ONLY
+ DEOPT_IF(!current_executor->base.vm_data.valid);
+ }
+
 
 // END BYTECODES //
 

Python/ceval.c

@@ -1057,7 +1057,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 deoptimize:
  // On DEOPT_IF we just repeat the last instruction.
  // This presumes nothing was popped from the stack (nor pushed).
- DPRINTF(2, "DEOPT: [Opcode %d, operand %" PRIu64 "]\n", opcode, operand);
+ DPRINTF(2, "DEOPT: [Opcode %d, operand %" PRIu64 " @ %d]\n", opcode, operand, (int)(next_uop-current_executor->trace-1));
  OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
  UOP_STAT_INC(opcode, miss);
  frame->return_offset = 0; // Dispatch to frame->instr_ptr

Python/optimizer.c

@@ -472,8 +472,8 @@ translate_bytecode_to_trace(
  } \
  reserved = (n); // Keep ADD_TO_TRACE / ADD_TO_STUB honest
 
-// Reserve space for main+stub uops, plus 2 for _SET_IP and _EXIT_TRACE
-#define RESERVE(main, stub) RESERVE_RAW((main) + (stub) + 2, uop_name(opcode))
+// Reserve space for main+stub uops, plus 3 for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
+#define RESERVE(main, stub) RESERVE_RAW((main) + (stub) + 3, uop_name(opcode))
 
 // Trace stack operations (used by _PUSH_FRAME, _POP_FRAME)
 #define TRACE_STACK_PUSH() \
@@ -503,8 +503,9 @@ translate_bytecode_to_trace(
 
 top: // Jump here after _PUSH_FRAME or likely branches
  for (;){
- RESERVE_RAW(2, "epilogue"); // Always need space for _SET_IP and _EXIT_TRACE
+ RESERVE_RAW(3, "epilogue"); // Always need space for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
  ADD_TO_TRACE(_SET_IP, INSTR_IP(instr, code), 0);
+ ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0);
 
  uint32_t opcode = instr->op.code;
  uint32_t oparg = instr->op.arg;

Python/optimizer_analysis.c

@@ -12,13 +12,13 @@
 #include <stddef.h>
 #include "pycore_optimizer.h"
 
-
 static void
 remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
 {
  // Note that we don't enter stubs, those SET_IPs are needed.
  int last_set_ip = -1;
  bool need_ip = true;
+ bool maybe_invalid = false;
  for (int pc = 0; pc < buffer_size; pc++){
  int opcode = buffer[pc].opcode;
  if (opcode == _SET_IP){
@@ -28,6 +28,16 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
  need_ip = false;
  last_set_ip = pc;
  }
+ else if (opcode == _CHECK_VALIDITY){
+ if (maybe_invalid){
+ /* Exiting the trace requires that IP is correct */
+ need_ip = true;
+ maybe_invalid = false;
+ }
+ else{
+ buffer[pc].opcode = NOP;
+ }
+ }
  else if (opcode == _JUMP_TO_TOP || opcode == _EXIT_TRACE){
  break;
  }
@@ -36,6 +46,9 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
  if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG) || opcode == _PUSH_FRAME){
  need_ip = true;
  }
+ if (_PyOpcode_opcode_metadata[opcode].flags & HAS_ESCAPES_FLAG){
+ maybe_invalid = true;
+ }
  }
  }
 }

Tools/cases_generator/analysis.py

@@ -390,7 +390,7 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction:
  else:
  targets.append(self.macro_instrs[target_name])
  assert targets
- ignored_flags ={"HAS_EVAL_BREAK_FLAG", "HAS_DEOPT_FLAG", "HAS_ERROR_FLAG"}
+ ignored_flags ={"HAS_EVAL_BREAK_FLAG", "HAS_DEOPT_FLAG", "HAS_ERROR_FLAG", "HAS_ESCAPES_FLAG"}
  assert len({t.instr_flags.bitmap(ignore=ignored_flags) for t in targets}) == 1
  return PseudoInstruction(pseudo.name, targets, targets[0].instr_flags)
 

Tools/cases_generator/flags.py

@@ -5,6 +5,55 @@
 import parsing
 from typing import AbstractSet
 
+WHITELIST = (
+ "Py_INCREF",
+ "_PyDictOrValues_IsValues",
+ "_PyObject_DictOrValuesPointer",
+ "_PyDictOrValues_GetValues",
+ "_PyObject_MakeInstanceAttributesFromDict",
+ "Py_DECREF",
+ "_Py_DECREF_SPECIALIZED",
+ "DECREF_INPUTS_AND_REUSE_FLOAT",
+ "PyUnicode_Append",
+ "_PyLong_IsZero",
+ "Py_SIZE",
+ "Py_TYPE",
+ "PyList_GET_ITEM",
+ "PyTuple_GET_ITEM",
+ "PyList_GET_SIZE",
+ "PyTuple_GET_SIZE",
+ "Py_ARRAY_LENGTH",
+ "Py_Unicode_GET_LENGTH",
+ "PyUnicode_READ_CHAR",
+ "_Py_SINGLETON",
+ "PyUnicode_GET_LENGTH",
+ "_PyLong_IsCompact",
+ "_PyLong_IsNonNegativeCompact",
+ "_PyLong_CompactValue",
+ "_Py_NewRef",
+)
+
+def makes_escaping_api_call(instr: parsing.Node) -> bool:
+ tkns = iter(instr.tokens)
+ for tkn in tkns:
+ if tkn.kind != lx.IDENTIFIER:
+ continue
+ try:
+ next_tkn = next(tkns)
+ except StopIteration:
+ return False
+ if next_tkn.kind != lx.LPAREN:
+ continue
+ if not tkn.text.startswith("Py") and not tkn.text.startswith("_Py"):
+ continue
+ if tkn.text.endswith("Check"):
+ continue
+ if tkn.text.endswith("CheckExact"):
+ continue
+ if tkn.text in WHITELIST:
+ continue
+ return True
+ return False
 
 @dataclasses.dataclass
 class InstructionFlags:
@@ -19,6 +68,7 @@ class InstructionFlags:
  HAS_EVAL_BREAK_FLAG: bool = False
  HAS_DEOPT_FLAG: bool = False
  HAS_ERROR_FLAG: bool = False
+ HAS_ESCAPES_FLAG: bool = False
 
  def __post_init__(self) -> None:
  self.bitmask ={name: (1 << i) for i, name in enumerate(self.names())}
@@ -50,6 +100,10 @@ def fromInstruction(instr: parsing.Node) -> "InstructionFlags":
  or variable_used(instr, "exception_unwind")
  or variable_used(instr, "resume_with_error")
  ),
+ HAS_ESCAPES_FLAG=(
+ variable_used(instr, "tstate")
+ or makes_escaping_api_call(instr)
+ ),
  )
 
  @staticmethod