gh-148211: decompose _POP_TOP_LOAD_CONST_INLINE(_BORROW) in JIT (GH-148230)

Author: NekoAsakura

Lib/test/test_capi/test_opt.py

@@ -1470,8 +1470,8 @@ class Bar:
         opnames = list(iter_opnames(ex))
         self.assertEqual(res, TIER2_THRESHOLD * 2 + 2)
         call = opnames.index("_CALL_BUILTIN_FAST")
-        load_attr_top = opnames.index("_POP_TOP_LOAD_CONST_INLINE_BORROW", 0, call)
-        load_attr_bottom = opnames.index("_POP_TOP_LOAD_CONST_INLINE_BORROW", call)
+        load_attr_top = opnames.index("_LOAD_CONST_INLINE_BORROW", 0, call)
+        load_attr_bottom = opnames.index("_LOAD_CONST_INLINE_BORROW", call)
         self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1)
         self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2)
 
@@ -1493,8 +1493,8 @@ class Foo:
         self.assertIsNotNone(ex)
         self.assertEqual(res, TIER2_THRESHOLD * 2)
         call = opnames.index("_CALL_BUILTIN_FAST_WITH_KEYWORDS")
-        load_attr_top = opnames.index("_POP_TOP_LOAD_CONST_INLINE_BORROW", 0, call)
-        load_attr_bottom = opnames.index("_POP_TOP_LOAD_CONST_INLINE_BORROW", call)
+        load_attr_top = opnames.index("_LOAD_CONST_INLINE_BORROW", 0, call)
+        load_attr_bottom = opnames.index("_LOAD_CONST_INLINE_BORROW", call)
         self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1)
         self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2)
 

Python/optimizer_analysis.c

@@ -155,7 +155,7 @@ type_watcher_callback(PyTypeObject* type)
 }
 
 static PyObject *
-convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj, bool pop, bool insert)
+convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj, bool insert)
 {
     assert(inst->opcode == _LOAD_GLOBAL_MODULE || inst->opcode == _LOAD_GLOBAL_BUILTINS || inst->opcode == _LOAD_ATTR_MODULE);
     assert(PyDict_CheckExact(obj));
@@ -183,9 +183,9 @@ convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj, bool pop, bool i
         }
     } else {
         if (_Py_IsImmortal(res)) {
-            inst->opcode = pop ? _POP_TOP_LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE_BORROW;
+            inst->opcode = _LOAD_CONST_INLINE_BORROW;
         } else {
-            inst->opcode = pop ? _POP_TOP_LOAD_CONST_INLINE : _LOAD_CONST_INLINE;
+            inst->opcode = _LOAD_CONST_INLINE;
         }
         if (inst->oparg & 1) {
             assert(inst[1].opcode == _PUSH_NULL_CONDITIONAL);
@@ -340,10 +340,12 @@ optimize_to_bool(
     int truthiness = sym_truthiness(ctx, value);
     if (truthiness >= 0) {
         PyObject *load = truthiness ? Py_True : Py_False;
-        int opcode = insert_mode ?
-            _INSERT_1_LOAD_CONST_INLINE_BORROW :
-            _POP_TOP_LOAD_CONST_INLINE_BORROW;
-        ADD_OP(opcode, 0, (uintptr_t)load);
+        if (insert_mode) {
+            ADD_OP(_INSERT_1_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)load);
+        } else {
+            ADD_OP(_POP_TOP, 0, 0);
+            ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)load);
+        }
         *result_ptr = sym_new_const(ctx, load);
         return 1;
     }
@@ -389,19 +391,22 @@ eliminate_pop_guard(_PyUOpInstruction *this_instr, JitOptContext *ctx, bool exit
 
 static JitOptRef
 lookup_attr(JitOptContext *ctx, _PyBloomFilter *dependencies, _PyUOpInstruction *this_instr,
-            PyTypeObject *type, PyObject *name, uint16_t immortal,
-            uint16_t mortal)
+            PyTypeObject *type, PyObject *name, bool pop)
 {
     // The cached value may be dead, so we need to do the lookup again... :(
     if (type && PyType_Check(type)) {
         PyObject *lookup = _PyType_Lookup(type, name);
         if (lookup) {
-            int opcode = mortal;
-            // if the object is immortal or the type is immutable, borrowing is safe
-            if (_Py_IsImmortal(lookup) || (type->tp_flags & Py_TPFLAGS_IMMUTABLETYPE)) {
-                opcode = immortal;
+            bool immortal = _Py_IsImmortal(lookup) || (type->tp_flags & Py_TPFLAGS_IMMUTABLETYPE);
+            if (pop) {
+                ADD_OP(_POP_TOP, 0, 0);
+                ADD_OP(immortal ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE,
+                       0, (uintptr_t)lookup);
+            }
+            else {
+                ADD_OP(immortal ? _INSERT_1_LOAD_CONST_INLINE_BORROW : _INSERT_1_LOAD_CONST_INLINE,
+                       0, (uintptr_t)lookup);
             }
-            ADD_OP(opcode, 0, (uintptr_t)lookup);
             PyType_Watch(TYPE_WATCHER_ID, (PyObject *)type);
             _Py_BloomFilter_Add(dependencies, type);
             return sym_new_const(ctx, lookup);

Python/optimizer_bytecodes.c

@@ -757,14 +757,6 @@ dummy_func(void) {
         value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
     }
 
-    op(_POP_TOP_LOAD_CONST_INLINE, (ptr/4, pop -- value)) {
-        value = sym_new_const(ctx, ptr);
-    }
-
-    op(_POP_TOP_LOAD_CONST_INLINE_BORROW, (ptr/4, pop -- value)) {
-        value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
-    }
-
     op(_POP_CALL_LOAD_CONST_INLINE_BORROW, (ptr/4, unused, unused -- value)) {
         value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
     }
@@ -845,7 +837,7 @@ dummy_func(void) {
                 if (watched_mutations < _Py_MAX_ALLOWED_GLOBALS_MODIFICATIONS) {
                     PyDict_Watch(GLOBALS_WATCHER_ID, dict);
                     _Py_BloomFilter_Add(dependencies, dict);
-                    PyObject *res = convert_global_to_const(this_instr, dict, false, true);
+                    PyObject *res = convert_global_to_const(this_instr, dict, true);
                     if (res == NULL) {
                         attr = sym_new_not_null(ctx);
                     }
@@ -898,35 +890,31 @@ dummy_func(void) {
         PyTypeObject *type = (PyTypeObject *)sym_get_const(ctx, owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _POP_TOP_LOAD_CONST_INLINE_BORROW,
-                           _POP_TOP_LOAD_CONST_INLINE);
+                           true);
     }
 
     op(_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES, (descr/4, owner -- attr)) {
         (void)descr;
         PyTypeObject *type = sym_get_type(owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _POP_TOP_LOAD_CONST_INLINE_BORROW,
-                           _POP_TOP_LOAD_CONST_INLINE);
+                           true);
     }
 
     op(_LOAD_ATTR_NONDESCRIPTOR_NO_DICT, (descr/4, owner -- attr)) {
         (void)descr;
         PyTypeObject *type = sym_get_type(owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _POP_TOP_LOAD_CONST_INLINE_BORROW,
-                           _POP_TOP_LOAD_CONST_INLINE);
+                           true);
     }
 
     op(_LOAD_ATTR_METHOD_WITH_VALUES, (descr/4, owner -- attr, self)) {
         (void)descr;
         PyTypeObject *type = sym_get_type(owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _INSERT_1_LOAD_CONST_INLINE_BORROW,
-                           _INSERT_1_LOAD_CONST_INLINE);
+                           false);
         self = owner;
     }
 
@@ -935,8 +923,7 @@ dummy_func(void) {
         PyTypeObject *type = sym_get_type(owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _INSERT_1_LOAD_CONST_INLINE_BORROW,
-                           _INSERT_1_LOAD_CONST_INLINE);
+                           false);
         self = owner;
     }
 
@@ -945,8 +932,7 @@ dummy_func(void) {
         PyTypeObject *type = sym_get_type(owner);
         PyObject *name = get_co_name(ctx, oparg >> 1);
         attr = lookup_attr(ctx, dependencies, this_instr, type, name,
-                           _INSERT_1_LOAD_CONST_INLINE_BORROW,
-                           _INSERT_1_LOAD_CONST_INLINE);
+                           false);
         self = owner;
     }
 
@@ -2010,7 +1996,7 @@ dummy_func(void) {
                 ctx->builtins_watched = true;
             }
             if (ctx->frame->globals_checked_version != 0 && ctx->frame->globals_watched) {
-                cnst = convert_global_to_const(this_instr, builtins, false, false);
+                cnst = convert_global_to_const(this_instr, builtins, false);
             }
         }
         if (cnst == NULL) {
@@ -2049,7 +2035,7 @@ dummy_func(void) {
                     ctx->frame->globals_checked_version = version;
                 }
                 if (ctx->frame->globals_checked_version == version) {
-                    cnst = convert_global_to_const(this_instr, globals, false, false);
+                    cnst = convert_global_to_const(this_instr, globals, false);
                 }
             }
         }

Python/optimizer_cases.c.h

@@ -238,18 +238,23 @@ def replace_opcode_if_evaluates_pure(
             # Map input count to output index (from TOS) and the appropriate constant-loading uop
             input_count_to_uop = {
                 1: {
-                    # (a -- a), usually for unary ops
-                    0: "_POP_TOP_LOAD_CONST_INLINE_BORROW",
+                    # (a -- res), usually for unary ops
+                    0: [("_POP_TOP", "0, 0"),
+                        ("_LOAD_CONST_INLINE_BORROW",
+                         "0, (uintptr_t)result")],
                     # (left -- res, left)
                     # usually for unary ops with passthrough references
-                    1: "_INSERT_1_LOAD_CONST_INLINE_BORROW",
+                    1: [("_INSERT_1_LOAD_CONST_INLINE_BORROW",
+                         "0, (uintptr_t)result")],
                 },
                 2: {
-                    # (a. b -- res), usually for binary ops
-                    0: "_POP_TWO_LOAD_CONST_INLINE_BORROW",
+                    # (a, b -- res), usually for binary ops
+                    0: [("_POP_TWO_LOAD_CONST_INLINE_BORROW",
+                         "0, (uintptr_t)result")],
                     # (left, right -- res, left, right)
                     # usually for binary ops with passthrough references
-                    2: "_INSERT_2_LOAD_CONST_INLINE_BORROW",
+                    2: [("_INSERT_2_LOAD_CONST_INLINE_BORROW",
+                         "0, (uintptr_t)result")],
                 },
             }
 
@@ -263,14 +268,16 @@ def replace_opcode_if_evaluates_pure(
             assert output_index >= 0
             input_count = len(used_stack_inputs)
             if input_count in input_count_to_uop and output_index in input_count_to_uop[input_count]:
-                replacement_uop = input_count_to_uop[input_count][output_index]
+                ops = input_count_to_uop[input_count][output_index]
                 input_desc = "one input" if input_count == 1 else "two inputs"
+                ops_desc = " + ".join(op for op, _ in ops)
 
                 emitter.emit(f"if (sym_is_const(ctx, {output_identifier.text})) {{\n")
                 emitter.emit(f"PyObject *result = sym_get_const(ctx, {output_identifier.text});\n")
                 emitter.emit(f"if (_Py_IsImmortal(result)) {{\n")
-                emitter.emit(f"// Replace with {replacement_uop} since we have {input_desc} and an immortal result\n")
-                emitter.emit(f"ADD_OP({replacement_uop}, 0, (uintptr_t)result);\n")
+                emitter.emit(f"// Replace with {ops_desc} since we have {input_desc} and an immortal result\n")
+                for op, args in ops:
+                    emitter.emit(f"ADD_OP({op}, {args});\n")
                 emitter.emit("}\n")
                 emitter.emit("}\n")