GH-130415: Improve the JIT's unneeded uop removal pass (GH-132333)

diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py
index 34b7c5982245c7524c702b0207694d7d47229cef..7fb505f27dfb9ed3d2a97576a9040341bae97e59 100644 (file)
--- a/Lib/test/test_capi/test_opt.py
+++ b/Lib/test/test_capi/test_opt.py
@@ -1283,7 +1283,7 @@ class TestUopsOptimization(unittest.TestCase):
         load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call)
         load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call)
         self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1)
-        self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1)
+        self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2)
 
     def test_guard_type_version_removed_escaping(self):
 
@@ -1306,7 +1306,7 @@ class TestUopsOptimization(unittest.TestCase):
         load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call)
         load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call)
         self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1)
-        self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1)
+        self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2)
 
     def test_guard_type_version_executor_invalidated(self):
         """
@@ -1601,7 +1601,7 @@ class TestUopsOptimization(unittest.TestCase):
         self.assertIsNotNone(ex)
         uops = get_opnames(ex)
         self.assertNotIn("_COMPARE_OP_INT", uops)
-        self.assertIn("_POP_TWO_LOAD_CONST_INLINE_BORROW", uops)
+        self.assertNotIn("_POP_TWO_LOAD_CONST_INLINE_BORROW", uops)
 
     def test_to_bool_bool_contains_op_set(self):
         """

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst
new file mode 100644 (file)
index 0000000..03523de
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst
@@ -0,0 +1,3 @@
+Improve the JIT's ability to remove unused constant and local variable
+loads, and fix an issue where deallocating unused values could cause JIT
+code to crash or behave incorrectly.

diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c
index ab28fae94a96a9ae3807dace445deb9b69319c39..387ebc813abd6677565d08a5ad12c0824869767b 100644 (file)
--- a/Python/optimizer_analysis.c
+++ b/Python/optimizer_analysis.c
@@ -555,28 +555,47 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
                 }
                 break;
             case _POP_TOP:
+            case _POP_TOP_LOAD_CONST_INLINE:
+            case _POP_TOP_LOAD_CONST_INLINE_BORROW:
+            case _POP_TWO_LOAD_CONST_INLINE_BORROW:
+            optimize_pop_top_again:
             {
                 _PyUOpInstruction *last = &buffer[pc-1];
                 while (last->opcode == _NOP) {
                     last--;
                 }
-                if (last->opcode == _LOAD_CONST_INLINE  ||
-                    last->opcode == _LOAD_CONST_INLINE_BORROW ||
-                    last->opcode == _LOAD_FAST ||
-                    last->opcode == _LOAD_FAST_BORROW ||
-                    last->opcode == _COPY
-                ) {
-                    last->opcode = _NOP;
-                    buffer[pc].opcode = _NOP;
-                }
-                if (last->opcode == _REPLACE_WITH_TRUE) {
-                    last->opcode = _NOP;
+                switch (last->opcode) {
+                    case _POP_TWO_LOAD_CONST_INLINE_BORROW:
+                        last->opcode = _POP_TOP;
+                        break;
+                    case _POP_TOP_LOAD_CONST_INLINE:
+                    case _POP_TOP_LOAD_CONST_INLINE_BORROW:
+                        last->opcode = _NOP;
+                        goto optimize_pop_top_again;
+                    case _COPY:
+                    case _LOAD_CONST_INLINE:
+                    case _LOAD_CONST_INLINE_BORROW:
+                    case _LOAD_FAST:
+                    case _LOAD_FAST_BORROW:
+                    case _LOAD_SMALL_INT:
+                        last->opcode = _NOP;
+                        if (opcode == _POP_TOP) {
+                            opcode = buffer[pc].opcode = _NOP;
+                        }
+                        else if (opcode == _POP_TOP_LOAD_CONST_INLINE) {
+                            opcode = buffer[pc].opcode = _LOAD_CONST_INLINE;
+                        }
+                        else if (opcode == _POP_TOP_LOAD_CONST_INLINE_BORROW) {
+                            opcode = buffer[pc].opcode = _LOAD_CONST_INLINE_BORROW;
+                        }
+                        else {
+                            assert(opcode == _POP_TWO_LOAD_CONST_INLINE_BORROW);
+                            opcode = buffer[pc].opcode = _POP_TOP_LOAD_CONST_INLINE_BORROW;
+                            goto optimize_pop_top_again;
+                        }
                 }
-                break;
+                _Py_FALLTHROUGH;
             }
-            case _JUMP_TO_TOP:
-            case _EXIT_TRACE:
-                return pc + 1;
             default:
             {
                 /* _PUSH_FRAME doesn't escape or error, but it
@@ -591,7 +610,11 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
                     buffer[last_set_ip].opcode = _SET_IP;
                     last_set_ip = -1;
                 }
+                break;
             }
+            case _JUMP_TO_TOP:
+            case _EXIT_TRACE:
+                return pc + 1;
         }
     }
     Py_UNREACHABLE();

diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c
index c5d8b536bc63419b914051f7bab678377eb3b815..6f7f9b03d3bf0695930facb77e06fc0e1006f8e5 100644 (file)
--- a/Python/optimizer_bytecodes.c
+++ b/Python/optimizer_bytecodes.c
@@ -912,6 +912,7 @@ dummy_func(void) {
     }
 
     op(_REPLACE_WITH_TRUE, (value -- res)) {
+        REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)Py_True);
         res = sym_new_const(ctx, Py_True);
     }
 

diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h
index 828f0943a8db866adb6a02d8c25b006c55ddec33..983af081e2f8cdd16a65e91a77d797d470e35b7c 100644 (file)
--- a/Python/optimizer_cases.c.h
+++ b/Python/optimizer_cases.c.h
@@ -274,6 +274,7 @@
 
         case _REPLACE_WITH_TRUE: {
             JitOptSymbol *res;
+            REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)Py_True);
             res = sym_new_const(ctx, Py_True);
             stack_pointer[-1] = res;
             break;