From b348313e7a48811acacc293ac4b2c8b20b4c631b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 22 Feb 2024 14:48:25 +0000 Subject: [PATCH] GH-115651: Convert `LOAD_MODULE_ATTR` into `LOAD_INLINE_CONST` when the module is itself a constant. (GH-115711) --- Python/bytecodes.c | 4 +- Python/executor_cases.c.h | 4 +- Python/generated_cases.c.h | 4 +- Python/optimizer_analysis.c | 68 +++++++----- .../tier2_redundancy_eliminator_bytecodes.c | 100 ++++++++++++------ Python/tier2_redundancy_eliminator_cases.c.h | 97 +++++++++++------ 6 files changed, 182 insertions(+), 95 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 5835b80582b..7e2c9c4d6a6 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1932,11 +1932,11 @@ dummy_func( _LOAD_ATTR_INSTANCE_VALUE + unused/5; // Skip over rest of cache - op(_CHECK_ATTR_MODULE, (type_version/2, owner -- owner)) { + op(_CHECK_ATTR_MODULE, (dict_version/2, owner -- owner)) { DEOPT_IF(!PyModule_CheckExact(owner)); PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict; assert(dict != NULL); - DEOPT_IF(dict->ma_keys->dk_version != type_version); + DEOPT_IF(dict->ma_keys->dk_version != dict_version); } op(_LOAD_ATTR_MODULE, (index/1, owner -- attr, null if (oparg & 1))) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 974555cbba9..3054058cf44 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1856,11 +1856,11 @@ case _CHECK_ATTR_MODULE: { PyObject *owner; owner = stack_pointer[-1]; - uint32_t type_version = (uint32_t)CURRENT_OPERAND(); + uint32_t dict_version = (uint32_t)CURRENT_OPERAND(); if (!PyModule_CheckExact(owner)) goto deoptimize; PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict; assert(dict != NULL); - if (dict->ma_keys->dk_version != type_version) goto deoptimize; + if (dict->ma_keys->dk_version != dict_version) goto deoptimize; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7f46bc8916c..87579134146 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3712,11 +3712,11 @@ // _CHECK_ATTR_MODULE owner = stack_pointer[-1]; { - uint32_t type_version = read_u32(&this_instr[2].cache); + uint32_t dict_version = read_u32(&this_instr[2].cache); DEOPT_IF(!PyModule_CheckExact(owner), LOAD_ATTR); PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict; assert(dict != NULL); - DEOPT_IF(dict->ma_keys->dk_version != type_version, LOAD_ATTR); + DEOPT_IF(dict->ma_keys->dk_version != dict_version, LOAD_ATTR); } // _LOAD_ATTR_MODULE { diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 5d2df9aca4e..68ef8254b49 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -271,6 +271,20 @@ sym_is_null(_Py_UOpsSymType *sym) return (sym->flags & (IS_NULL | NOT_NULL)) == IS_NULL; } +static inline bool +sym_is_const(_Py_UOpsSymType *sym) +{ + return (sym->flags & TRUE_CONST) != 0; +} + +static inline PyObject * +sym_get_const(_Py_UOpsSymType *sym) +{ + assert(sym_is_const(sym)); + assert(sym->const_val); + return sym->const_val; +} + static inline void sym_set_type(_Py_UOpsSymType *sym, PyTypeObject *tp) { @@ -336,18 +350,6 @@ sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) return temp; } -static inline bool -is_const(_Py_UOpsSymType *sym) -{ - return sym->const_val != NULL; -} - -static inline PyObject * -get_const(_Py_UOpsSymType *sym) -{ - return sym->const_val; -} - static _Py_UOpsSymType* sym_new_null(_Py_UOpsAbstractInterpContext *ctx) { @@ -408,18 +410,21 @@ globals_watcher_callback(PyDict_WatchEvent event, PyObject* dict, return 0; } -static void -global_to_const(_PyUOpInstruction *inst, PyObject *obj) +static PyObject * +convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj) { - assert(inst->opcode == _LOAD_GLOBAL_MODULE || inst->opcode == _LOAD_GLOBAL_BUILTINS); + assert(inst->opcode == _LOAD_GLOBAL_MODULE || inst->opcode == _LOAD_GLOBAL_BUILTINS || inst->opcode == _LOAD_ATTR_MODULE); assert(PyDict_CheckExact(obj)); PyDictObject *dict = (PyDictObject *)obj; assert(dict->ma_keys->dk_kind == DICT_KEYS_UNICODE); PyDictUnicodeEntry *entries = DK_UNICODE_ENTRIES(dict->ma_keys); assert(inst->operand <= UINT16_MAX); + if ((int)inst->operand >= dict->ma_keys->dk_nentries) { + return NULL; + } PyObject *res = entries[inst->operand].me_value; if (res == NULL) { - return; + return NULL; } if (_Py_IsImmortal(res)) { inst->opcode = (inst->oparg & 1) ? _LOAD_CONST_INLINE_BORROW_WITH_NULL : _LOAD_CONST_INLINE_BORROW; @@ -428,6 +433,7 @@ global_to_const(_PyUOpInstruction *inst, PyObject *obj) inst->opcode = (inst->oparg & 1) ? _LOAD_CONST_INLINE_WITH_NULL : _LOAD_CONST_INLINE; } inst->operand = (uint64_t)res; + return res; } static int @@ -524,12 +530,12 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer, break; case _LOAD_GLOBAL_BUILTINS: if (globals_checked & builtins_checked & globals_watched & builtins_watched & 1) { - global_to_const(inst, builtins); + convert_global_to_const(inst, builtins); } break; case _LOAD_GLOBAL_MODULE: if (globals_checked & globals_watched & 1) { - global_to_const(inst, globals); + convert_global_to_const(inst, globals); } break; case _PUSH_FRAME: @@ -603,7 +609,8 @@ uop_redundancy_eliminator( PyCodeObject *co, _PyUOpInstruction *trace, int trace_len, - int curr_stacklen + int curr_stacklen, + _PyBloomFilter *dependencies ) { @@ -665,7 +672,7 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) * could error. _CHECK_VALIDITY is needed if the previous * instruction could have escaped. */ int last_set_ip = -1; - bool may_have_escaped = false; + bool may_have_escaped = true; for (int pc = 0; pc < buffer_size; pc++) { int opcode = buffer[pc].opcode; switch (opcode) { @@ -691,6 +698,22 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) } last_set_ip = pc; break; + case _POP_TOP: + { + _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 == _COPY + ) { + last->opcode = _NOP; + buffer[pc].opcode = NOP; + } + break; + } case _JUMP_TO_TOP: case _EXIT_TRACE: return; @@ -704,9 +727,6 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) if (_PyUop_Flags[opcode] & HAS_ERROR_FLAG) { needs_ip = true; } - if (opcode == _PUSH_FRAME) { - needs_ip = true; - } if (needs_ip && last_set_ip >= 0) { if (buffer[last_set_ip].opcode == _CHECK_VALIDITY) { buffer[last_set_ip].opcode = _CHECK_VALIDITY_AND_SET_IP; @@ -791,7 +811,7 @@ _Py_uop_analyze_and_optimize( err = uop_redundancy_eliminator( (PyCodeObject *)frame->f_executable, buffer, - buffer_size, curr_stacklen); + buffer_size, curr_stacklen, dependencies); if (err == 0) { goto not_ready; diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c index e9b556d16c3..ff2b9a42272 100644 --- a/Python/tier2_redundancy_eliminator_bytecodes.c +++ b/Python/tier2_redundancy_eliminator_bytecodes.c @@ -1,6 +1,7 @@ #include "Python.h" #include "pycore_uops.h" #include "pycore_uop_ids.h" +#include "internal/pycore_moduleobject.h" #define op(name, ...) /* NAME is ignored */ @@ -87,11 +88,11 @@ dummy_func(void) { } op(_BINARY_OP_ADD_INT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Add((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Add((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -105,11 +106,11 @@ dummy_func(void) { } op(_BINARY_OP_SUBTRACT_INT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Subtract((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Subtract((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -123,11 +124,11 @@ dummy_func(void) { } op(_BINARY_OP_MULTIPLY_INT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Multiply((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Multiply((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -141,12 +142,12 @@ dummy_func(void) { } op(_BINARY_OP_ADD_FLOAT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) + - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) + + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -160,12 +161,12 @@ dummy_func(void) { } op(_BINARY_OP_SUBTRACT_FLOAT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) - - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) - + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -179,12 +180,12 @@ dummy_func(void) { } op(_BINARY_OP_MULTIPLY_FLOAT, (left, right -- res)) { - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) * - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) * + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -237,10 +238,43 @@ dummy_func(void) { (void)owner; } + op(_CHECK_ATTR_MODULE, (dict_version/2, owner -- owner)) { + (void)dict_version; + if (sym_is_const(owner)) { + PyObject *cnst = sym_get_const(owner); + if (PyModule_CheckExact(cnst)) { + PyModuleObject *mod = (PyModuleObject *)cnst; + PyObject *dict = mod->md_dict; + uint64_t watched_mutations = get_mutations(dict); + if (watched_mutations < _Py_MAX_ALLOWED_GLOBALS_MODIFICATIONS) { + PyDict_Watch(GLOBALS_WATCHER_ID, dict); + _Py_BloomFilter_Add(dependencies, dict); + this_instr->opcode = _NOP; + } + } + } + } + op(_LOAD_ATTR_MODULE, (index/1, owner -- attr, null if (oparg & 1))) { - _LOAD_ATTR_NOT_NULL (void)index; - (void)owner; + OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); + attr = NULL; + if (this_instr[-1].opcode == _NOP) { + // Preceding _CHECK_ATTR_MODULE was removed: mod is const and dict is watched. + assert(sym_is_const(owner)); + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + assert(PyModule_CheckExact(mod)); + PyObject *dict = mod->md_dict; + PyObject *res = convert_global_to_const(this_instr, dict); + if (res != NULL) { + this_instr[-1].opcode = _POP_TOP; + OUT_OF_SPACE_IF_NULL(attr = sym_new_const(ctx, res)); + } + } + if (attr == NULL) { + /* No conversion made. We don't know what `attr` is. */ + OUT_OF_SPACE_IF_NULL(attr = sym_new_known_notnull(ctx)); + } } op(_LOAD_ATTR_WITH_HINT, (hint/1, owner -- attr, null if (oparg & 1))) { @@ -347,4 +381,4 @@ dummy_func(void) { // END BYTECODES // -} \ No newline at end of file +} diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h index f41fe328195..58c11b7a1c8 100644 --- a/Python/tier2_redundancy_eliminator_cases.c.h +++ b/Python/tier2_redundancy_eliminator_cases.c.h @@ -183,11 +183,11 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Multiply((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Multiply((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -209,11 +209,11 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Add((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Add((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -235,11 +235,11 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyLong_CheckExact(get_const(left))); - assert(PyLong_CheckExact(get_const(right))); - PyObject *temp = _PyLong_Subtract((PyLongObject *)get_const(left), - (PyLongObject *)get_const(right)); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyLong_CheckExact(sym_get_const(left))); + assert(PyLong_CheckExact(sym_get_const(right))); + PyObject *temp = _PyLong_Subtract((PyLongObject *)sym_get_const(left), + (PyLongObject *)sym_get_const(right)); if (temp == NULL) { goto error; } @@ -275,12 +275,12 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) * - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) * + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -302,12 +302,12 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) + - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) + + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -329,12 +329,12 @@ _Py_UOpsSymType *res; right = stack_pointer[-1]; left = stack_pointer[-2]; - if (is_const(left) && is_const(right)) { - assert(PyFloat_CheckExact(get_const(left))); - assert(PyFloat_CheckExact(get_const(right))); + if (sym_is_const(left) && sym_is_const(right)) { + assert(PyFloat_CheckExact(sym_get_const(left))); + assert(PyFloat_CheckExact(sym_get_const(right))); PyObject *temp = PyFloat_FromDouble( - PyFloat_AS_DOUBLE(get_const(left)) - - PyFloat_AS_DOUBLE(get_const(right))); + PyFloat_AS_DOUBLE(sym_get_const(left)) - + PyFloat_AS_DOUBLE(sym_get_const(right))); if (temp == NULL) { goto error; } @@ -898,6 +898,23 @@ } case _CHECK_ATTR_MODULE: { + _Py_UOpsSymType *owner; + owner = stack_pointer[-1]; + uint32_t dict_version = (uint32_t)this_instr->operand; + (void)dict_version; + if (sym_is_const(owner)) { + PyObject *cnst = sym_get_const(owner); + if (PyModule_CheckExact(cnst)) { + PyModuleObject *mod = (PyModuleObject *)cnst; + PyObject *dict = mod->md_dict; + uint64_t watched_mutations = get_mutations(dict); + if (watched_mutations < _Py_MAX_ALLOWED_GLOBALS_MODIFICATIONS) { + PyDict_Watch(GLOBALS_WATCHER_ID, dict); + _Py_BloomFilter_Add(dependencies, dict); + this_instr->opcode = _NOP; + } + } + } break; } @@ -907,9 +924,25 @@ _Py_UOpsSymType *null = NULL; owner = stack_pointer[-1]; uint16_t index = (uint16_t)this_instr->operand; - _LOAD_ATTR_NOT_NULL (void)index; - (void)owner; + OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); + attr = NULL; + if (this_instr[-1].opcode == _NOP) { + // Preceding _CHECK_ATTR_MODULE was removed: mod is const and dict is watched. + assert(sym_is_const(owner)); + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + assert(PyModule_CheckExact(mod)); + PyObject *dict = mod->md_dict; + PyObject *res = convert_global_to_const(this_instr, dict); + if (res != NULL) { + this_instr[-1].opcode = _POP_TOP; + OUT_OF_SPACE_IF_NULL(attr = sym_new_const(ctx, res)); + } + } + if (attr == NULL) { + /* No conversion made. We don't know what `attr` is. */ + OUT_OF_SPACE_IF_NULL(attr = sym_new_known_notnull(ctx)); + } stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1);