diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 124a2c0115769c..5d512f19b96933 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/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) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 3f0aabed10e1db..ed1619df2a188b 100644 --- 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 7a0aac11890306..e10891bf9697af 100644 --- 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 82ac3c84f02880..5c56a9babd889c 100644 --- 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 65896221ba7a05..495462b6bcd906 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -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")