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

diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py
index 124a2c0115769c92b6ad43661aa5ed82d27e8c91..5d512f19b969333720b52f8ed02d3fb51ccaa110 100644 (file)
--- a/Lib/test/test_capi/test_opt.py
+++ b/Lib/test/test_capi/test_opt.py
@@ -1470,8 +1470,8 @@ class TestUopsOptimization(unittest.TestCase):
         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 TestUopsOptimization(unittest.TestCase):
         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)
 

diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c
index 3f0aabed10e1db6f1af66b68eb9c87d6cb212ba3..ed1619df2a188b447c2a9ebaa017d63ced46d0b3 100644 (file)
--- a/Python/optimizer_analysis.c
+++ b/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);

diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c
index 7a0aac1189030632cdfe480dac1b660bdf8799a5..e10891bf9697afb835ca6686a9f8c45d6222bc41 100644 (file)
--- a/Python/optimizer_bytecodes.c
+++ b/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,8 +890,7 @@ 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)) {
@@ -907,8 +898,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,
-                           _POP_TOP_LOAD_CONST_INLINE_BORROW,
-                           _POP_TOP_LOAD_CONST_INLINE);
+                           true);
     }
 
     op(_LOAD_ATTR_NONDESCRIPTOR_NO_DICT, (descr/4, owner -- attr)) {
@@ -916,8 +906,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,
-                           _POP_TOP_LOAD_CONST_INLINE_BORROW,
-                           _POP_TOP_LOAD_CONST_INLINE);
+                           true);
     }
 
     op(_LOAD_ATTR_METHOD_WITH_VALUES, (descr/4, owner -- attr, self)) {
@@ -925,8 +914,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;
     }
 
@@ -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);
                 }
             }
         }

diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h
index 82ac3c84f028801bcecb2f668e309b44f7a8e554..5c56a9babd889c58f38cb9ed2bf328d607fe1907 100644 (file)
--- a/Python/optimizer_cases.c.h
+++ b/Python/optimizer_cases.c.h
@@ -314,8 +314,9 @@
                 if (sym_is_const(ctx, res)) {
                     PyObject *result = sym_get_const(ctx, res);
                     if (_Py_IsImmortal(result)) {
-                        // Replace with _POP_TOP_LOAD_CONST_INLINE_BORROW since we have one input and an immortal result
-                        ADD_OP(_POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)result);
+                        // Replace with _POP_TOP + _LOAD_CONST_INLINE_BORROW since we have one input and an immortal result
+                        ADD_OP(_POP_TOP, 0, 0);
+                        ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)result);
                     }
                 }
                 stack_pointer[-1] = res;
@@ -2085,7 +2086,7 @@
                         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);
                     }
                 }
             }
@@ -2128,7 +2129,7 @@
                     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) {
@@ -2438,7 +2439,7 @@
                     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);
                         }
@@ -2519,8 +2520,7 @@
             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);
             stack_pointer[-1] = attr;
             break;
         }
@@ -3413,8 +3413,7 @@
             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;
             CHECK_STACK_BOUNDS(1);
             stack_pointer[-1] = attr;
@@ -3434,8 +3433,7 @@
             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;
             CHECK_STACK_BOUNDS(1);
             stack_pointer[-1] = attr;
@@ -3454,8 +3452,7 @@
             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);
             stack_pointer[-1] = attr;
             break;
         }
@@ -3469,8 +3466,7 @@
             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);
             stack_pointer[-1] = attr;
             break;
         }
@@ -3489,8 +3485,7 @@
             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;
             CHECK_STACK_BOUNDS(1);
             stack_pointer[-1] = attr;
@@ -4857,8 +4852,7 @@
 
         case _POP_TOP_LOAD_CONST_INLINE: {
             JitOptRef value;
-            PyObject *ptr = (PyObject *)this_instr->operand0;
-            value = sym_new_const(ctx, ptr);
+            value = sym_new_not_null(ctx);
             stack_pointer[-1] = value;
             break;
         }
@@ -4897,8 +4891,7 @@
 
         case _POP_TOP_LOAD_CONST_INLINE_BORROW: {
             JitOptRef value;
-            PyObject *ptr = (PyObject *)this_instr->operand0;
-            value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
+            value = sym_new_not_null(ctx);
             stack_pointer[-1] = value;
             break;
         }

diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py
index 65896221ba7a05a55a39aabcf4cd8da02420f2d3..495462b6bcd9065e749569427f0bb5230922500c 100644 (file)
--- a/Tools/cases_generator/optimizer_generator.py
+++ b/Tools/cases_generator/optimizer_generator.py
@@ -238,18 +238,23 @@ class OptimizerEmitter(Emitter):
             # 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 @@ class OptimizerEmitter(Emitter):
             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")