From 2762d4c2620876650b530f65f06f080f8a70c116 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 15 Feb 2024 15:12:26 +0000 Subject: [PATCH 1/4] Turn loads of attributes of modules to constants --- Python/bytecodes.c | 4 +- Python/executor_cases.c.h | 4 +- Python/generated_cases.c.h | 4 +- Python/optimizer_analysis.c | 26 ++++++++++--- .../tier2_redundancy_eliminator_bytecodes.c | 37 +++++++++++++++++-- Python/tier2_redundancy_eliminator_cases.c.h | 34 ++++++++++++++++- 6 files changed, 93 insertions(+), 16 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6822e772e913e8..baf0677c41bda5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1931,11 +1931,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 11e2a1fe85d51d..74a4243bd08887 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1660,11 +1660,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 6c19adc60c690f..ab71637578b611 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3692,11 +3692,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 b104d2fa7baec9..1446e3e735c325 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -276,6 +276,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) { @@ -413,10 +427,10 @@ globals_watcher_callback(PyDict_WatchEvent event, PyObject* dict, return 0; } -static void +static PyObject * 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); @@ -424,7 +438,7 @@ global_to_const(_PyUOpInstruction *inst, PyObject *obj) assert(inst->operand <= UINT16_MAX); 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; @@ -433,6 +447,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 @@ -608,7 +623,8 @@ uop_redundancy_eliminator( PyCodeObject *co, _PyUOpInstruction *trace, int trace_len, - int curr_stacklen + int curr_stacklen, + _PyBloomFilter *dependencies ) { @@ -796,7 +812,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 3f6e8ce1bbfbad..35f596f9c971be 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 */ @@ -229,10 +230,40 @@ dummy_func(void) { (void)owner; } + op(_CHECK_ATTR_MODULE, (dict_version/2, owner -- owner)) { + (void)dict_version; + if (sym_is_const(owner)) { + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + if (PyModule_CheckExact(mod)) { + 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) { + assert(sym_is_const(owner)); + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + assert(PyModule_CheckExact(mod)); + PyObject *dict = mod->md_dict; + PyObject *res = 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) { + OUT_OF_SPACE_IF_NULL(attr = sym_new_known_notnull(ctx)); + } } op(_LOAD_ATTR_WITH_HINT, (hint/1, owner -- attr, null if (oparg & 1))) { @@ -339,4 +370,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 be2fbb9106fffc..0c0284d76149ff 100644 --- a/Python/tier2_redundancy_eliminator_cases.c.h +++ b/Python/tier2_redundancy_eliminator_cases.c.h @@ -888,6 +888,22 @@ } 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)) { + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + if (PyModule_CheckExact(mod)) { + 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; } @@ -897,9 +913,23 @@ _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) { + assert(sym_is_const(owner)); + PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); + assert(PyModule_CheckExact(mod)); + PyObject *dict = mod->md_dict; + PyObject *res = 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) { + 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); From 76389dd13ef3d27cc351c6c1223b83d2c3756853 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 15 Feb 2024 19:43:55 +0000 Subject: [PATCH 2/4] Fix cleanup --- Python/optimizer_analysis.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 1446e3e735c325..98a6568452cd31 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -686,7 +686,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) { @@ -712,6 +712,21 @@ 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; + } + } case _JUMP_TO_TOP: case _EXIT_TRACE: return; @@ -725,9 +740,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; From 6d95a8b6284468d965bb65075902a63036760cda Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 21 Feb 2024 10:30:00 +0000 Subject: [PATCH 3/4] Address review comments --- Python/optimizer_analysis.c | 12 ---- .../tier2_redundancy_eliminator_bytecodes.c | 61 ++++++++++--------- Python/tier2_redundancy_eliminator_cases.c.h | 61 ++++++++++--------- 3 files changed, 62 insertions(+), 72 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 98a6568452cd31..6722bc825e6f08 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -355,18 +355,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) { diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c index 35f596f9c971be..8b72f99b0269a8 100644 --- a/Python/tier2_redundancy_eliminator_bytecodes.c +++ b/Python/tier2_redundancy_eliminator_bytecodes.c @@ -80,11 +80,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; } @@ -98,11 +98,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; } @@ -116,11 +116,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; } @@ -134,12 +134,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; } @@ -153,12 +153,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; } @@ -172,12 +172,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; } @@ -251,6 +251,7 @@ dummy_func(void) { 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)); diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h index 115405028c1043..44930827337576 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; } @@ -917,6 +917,7 @@ 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)); From e1f5fa99d98c220e15702780f623436be51b1815 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 21 Feb 2024 17:49:13 +0000 Subject: [PATCH 4/4] Address review comments --- Python/optimizer_analysis.c | 10 +++++++--- Python/tier2_redundancy_eliminator_bytecodes.c | 8 +++++--- Python/tier2_redundancy_eliminator_cases.c.h | 8 +++++--- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 6722bc825e6f08..a1f3fa944891b5 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -416,7 +416,7 @@ globals_watcher_callback(PyDict_WatchEvent event, PyObject* dict, } static PyObject * -global_to_const(_PyUOpInstruction *inst, PyObject *obj) +convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj) { assert(inst->opcode == _LOAD_GLOBAL_MODULE || inst->opcode == _LOAD_GLOBAL_BUILTINS || inst->opcode == _LOAD_ATTR_MODULE); assert(PyDict_CheckExact(obj)); @@ -424,6 +424,9 @@ global_to_const(_PyUOpInstruction *inst, PyObject *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 NULL; @@ -532,12 +535,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: @@ -714,6 +717,7 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) last->opcode = _NOP; buffer[pc].opcode = NOP; } + break; } case _JUMP_TO_TOP: case _EXIT_TRACE: diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c index 8b72f99b0269a8..c44e913822952c 100644 --- a/Python/tier2_redundancy_eliminator_bytecodes.c +++ b/Python/tier2_redundancy_eliminator_bytecodes.c @@ -233,8 +233,9 @@ dummy_func(void) { op(_CHECK_ATTR_MODULE, (dict_version/2, owner -- owner)) { (void)dict_version; if (sym_is_const(owner)) { - PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); - if (PyModule_CheckExact(mod)) { + 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) { @@ -256,13 +257,14 @@ dummy_func(void) { PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); assert(PyModule_CheckExact(mod)); PyObject *dict = mod->md_dict; - PyObject *res = global_to_const(this_instr, 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)); } } diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h index 44930827337576..de9d495e09a29a 100644 --- a/Python/tier2_redundancy_eliminator_cases.c.h +++ b/Python/tier2_redundancy_eliminator_cases.c.h @@ -893,8 +893,9 @@ uint32_t dict_version = (uint32_t)this_instr->operand; (void)dict_version; if (sym_is_const(owner)) { - PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); - if (PyModule_CheckExact(mod)) { + 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) { @@ -922,13 +923,14 @@ PyModuleObject *mod = (PyModuleObject *)sym_get_const(owner); assert(PyModule_CheckExact(mod)); PyObject *dict = mod->md_dict; - PyObject *res = global_to_const(this_instr, 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;