From 803e9f7e6f1eb3dd722d97a40c74e9a073fa24ab Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 18:23:04 +0800 Subject: [PATCH 01/17] deferred globals --- Include/internal/pycore_dict.h | 4 +- Objects/dictobject.c | 126 +++++++++++++++++++++++++++++---- Python/bytecodes.c | 59 +++++++++++---- Python/executor_cases.c.h | 41 +++++++++-- Python/generated_cases.c.h | 63 +++++++++++++---- Python/optimizer_cases.c.h | 8 ++- 6 files changed, 251 insertions(+), 50 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index a84246ee34efff..877b35af1a7169 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -10,6 +10,7 @@ extern "C" { #include "pycore_object.h" // PyManagedDictPointer #include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SSIZE_ACQUIRE +#include "pycore_stackref.h" // _PyStackRef // Unsafe flavor of PyDict_GetItemWithError(): no error checking extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key); @@ -100,10 +101,11 @@ extern void _PyDictKeys_DecRef(PyDictKeysObject *keys); */ extern Py_ssize_t _Py_dict_lookup(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); extern Py_ssize_t _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); +extern Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr); extern Py_ssize_t _PyDict_LookupIndex(PyDictObject *, PyObject *); extern Py_ssize_t _PyDictKeys_StringLookup(PyDictKeysObject* dictkeys, PyObject *key); -PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObject *); +PyAPI_FUNC(void)_PyDict_LoadGlobalStackRef(PyDictObject *, PyDictObject *, PyObject *, _PyStackRef *); /* Consumes references to key and value */ PyAPI_FUNC(int) _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value); diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 3e9f982ae070a3..b0b8efcaa88b9d 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1496,6 +1496,97 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb return ix; } +Py_ssize_t +_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +{ + PyDictKeysObject *dk; + DictKeysKind kind; + Py_ssize_t ix; + + ensure_shared_on_read(mp); + + dk = _Py_atomic_load_ptr(&mp->ma_keys); + kind = dk->dk_kind; + + if (kind != DICT_KEYS_GENERAL) { + if (PyUnicode_CheckExact(key)) { + ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + } + else { + ix = unicodekeys_lookup_generic_threadsafe(mp, dk, key, hash); + } + if (ix == DKIX_KEY_CHANGED) { + goto read_failed; + } + + if (ix >= 0) { + if (kind == DICT_KEYS_SPLIT) { + PyDictValues *values = _Py_atomic_load_ptr(&mp->ma_values); + if (values == NULL) + goto read_failed; + + uint8_t capacity = _Py_atomic_load_uint8_relaxed(&values->capacity); + if (ix >= (Py_ssize_t)capacity) + goto read_failed; + + *value_addr = PyStackRef_FromPyObjectNew(values->values[ix]); + if (PyStackRef_IsNull(*value_addr)) { + goto read_failed; + } + + if (values != _Py_atomic_load_ptr(&mp->ma_values)) { + goto read_failed; + } + } + else { + *value_addr = PyStackRef_FromPyObjectNew(DK_UNICODE_ENTRIES(dk)[ix].me_value); + if (PyStackRef_IsNull(*value_addr)) { + goto read_failed; + } + + if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { + goto read_failed; + } + } + } + else { + *value_addr = PyStackRef_NULL; + } + } + else { + ix = dictkeys_generic_lookup_threadsafe(mp, dk, key, hash); + if (ix == DKIX_KEY_CHANGED) { + goto read_failed; + } + if (ix >= 0) { + *value_addr = PyStackRef_FromPyObjectNew(DK_ENTRIES(dk)[ix].me_value); + if (PyStackRef_IsNull(*value_addr)) { + goto read_failed; + } + if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { + goto read_failed; + } + } + else { + *value_addr = PyStackRef_NULL; + } + } + + return ix; + + PyObject *value; +read_failed: + // In addition to the normal races of the dict being modified the _Py_TryXGetRef + // can all fail if they don't yet have a shared ref count. That can happen here + // or in the *_lookup_* helper. In that case we need to take the lock to avoid + // mutation and do a normal incref which will make them shared. + Py_BEGIN_CRITICAL_SECTION(mp); + ix = _Py_dict_lookup(mp, key, hash, &value); + *value_addr = value == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(value); + Py_END_CRITICAL_SECTION(); + return ix; +} + #else // Py_GIL_DISABLED Py_ssize_t @@ -1506,6 +1597,15 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb return ix; } +Py_ssize_t +_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +{ + PyObject *val; + Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); + *value_addr = PyStackRef_FromPyObjectNew(val); + return ix; +} + #endif int @@ -2391,31 +2491,31 @@ _PyDict_GetItemStringWithError(PyObject *v, const char *key) * key hash failed, key comparison failed, ...). Return NULL if the key doesn't * exist. Return the value if the key exists. * - * Returns a new reference. + * Stores a new stack reference. */ -PyObject * -_PyDict_LoadGlobal(PyDictObject *globals, PyDictObject *builtins, PyObject *key) +void +_PyDict_LoadGlobalStackRef(PyDictObject *globals, PyDictObject *builtins, PyObject *key, _PyStackRef *res) { Py_ssize_t ix; Py_hash_t hash; - PyObject *value; hash = _PyObject_HashFast(key); if (hash == -1) { - return NULL; + *res = PyStackRef_NULL; } /* namespace 1: globals */ - ix = _Py_dict_lookup_threadsafe(globals, key, hash, &value); - if (ix == DKIX_ERROR) - return NULL; - if (ix != DKIX_EMPTY && value != NULL) - return value; + ix = _Py_dict_lookup_threadsafe_stackref(globals, key, hash, res); + if (ix == DKIX_ERROR) { + *res = PyStackRef_NULL; + } + if (ix != DKIX_EMPTY && !PyStackRef_IsNull(*res)) { + return; + } /* namespace 2: builtins */ - ix = _Py_dict_lookup_threadsafe(builtins, key, hash, &value); - assert(ix >= 0 || value == NULL); - return value; + ix = _Py_dict_lookup_threadsafe_stackref(builtins, key, hash, res); + assert(ix >= 0 || PyStackRef_IsNull(*res)); } /* Consumes references to key and value */ diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 97e37bc28b8cfd..8fe5ecb4c5bd71 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1436,23 +1436,23 @@ dummy_func( locals = PyStackRef_FromPyObjectNew(l); } - inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { + inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v: _PyStackRef *)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - PyObject *v_o; - int err = PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o); - if (err < 0) { + PyObject *v_o = NULL; + if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { ERROR_NO_POP(); } if (v_o == NULL) { if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { - v_o = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), + _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), (PyDictObject *)BUILTINS(), - name); - if (v_o == NULL) { + name, + v); + if (PyStackRef_IsNull(*v)) { if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobal() returns NULL without raising + /* _PyDict_LoadGlobalStackRef() sets NULL without raising * an exception if the key doesn't exist */ _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, NAME_ERROR_MSG, name); @@ -1476,8 +1476,10 @@ dummy_func( } } } + if (v_o != NULL) { + *v = PyStackRef_FromPyObjectSteal(v_o); + } DECREF_INPUTS(); - v = PyStackRef_FromPyObjectSteal(v_o); } inst(LOAD_NAME, (-- v)) { @@ -1505,12 +1507,43 @@ dummy_func( #endif /* ENABLE_SPECIALIZATION */ } - op(_LOAD_GLOBAL, ( -- res, null if (oparg & 1))) { + op(_LOAD_GLOBAL, ( -- res[1], null if (oparg & 1))) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - PyObject *res_o = _PyEval_LoadGlobal(GLOBALS(), BUILTINS(), name); - ERROR_IF(res_o == NULL, error); + if (PyDict_CheckExact(GLOBALS()) + && PyDict_CheckExact(BUILTINS())) + { + _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name, + res); + if (PyStackRef_IsNull(*res)) { + if (!_PyErr_Occurred(tstate)) { + /* _PyDict_LoadGlobalStackRef() sets NULL without raising + * an exception if the key doesn't exist */ + _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + } + ERROR_IF(true, error); + } + } + else { + PyObject *res_o; + /* Slow-path if globals or builtins is not a dict */ + /* namespace 1: globals */ + ERROR_IF(PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0, error); + if (res_o == NULL) { + /* namespace 2: builtins */ + ERROR_IF(PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0, error); + if (res_o == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + ERROR_IF(true, error); + } + } + *res = PyStackRef_FromPyObjectSteal(res_o); + } null = PyStackRef_NULL; - res = PyStackRef_FromPyObjectSteal(res_o); } macro(LOAD_GLOBAL) = diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index b8785381cdd572..f7af64eb2e05ea 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1660,15 +1660,46 @@ } case _LOAD_GLOBAL: { - _PyStackRef res; + _PyStackRef *res; _PyStackRef null = PyStackRef_NULL; oparg = CURRENT_OPARG(); + res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - PyObject *res_o = _PyEval_LoadGlobal(GLOBALS(), BUILTINS(), name); - if (res_o == NULL) JUMP_TO_ERROR(); + if (PyDict_CheckExact(GLOBALS()) + && PyDict_CheckExact(BUILTINS())) + { + _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name, + res); + if (PyStackRef_IsNull(*res)) { + if (!_PyErr_Occurred(tstate)) { + /* _PyDict_LoadGlobalStackRef() sets NULL without raising + * an exception if the key doesn't exist */ + _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + } + if (true) JUMP_TO_ERROR(); + } + } + else { + PyObject *res_o; + /* Slow-path if globals or builtins is not a dict */ + /* namespace 1: globals */ + if (PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0) JUMP_TO_ERROR(); + if (res_o == NULL) { + /* namespace 2: builtins */ + if (PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0) JUMP_TO_ERROR(); + if (res_o == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + if (true) JUMP_TO_ERROR(); + } + } + *res = PyStackRef_FromPyObjectSteal(res_o); + } null = PyStackRef_NULL; - res = PyStackRef_FromPyObjectSteal(res_o); - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7582b06a7641aa..f23c19e564069d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5287,24 +5287,24 @@ next_instr += 1; INSTRUCTION_STATS(LOAD_FROM_DICT_OR_GLOBALS); _PyStackRef mod_or_class_dict; - _PyStackRef v; + _PyStackRef *v; mod_or_class_dict = stack_pointer[-1]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - PyObject *v_o; - int err = PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o); - if (err < 0) { + PyObject *v_o = NULL; + if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { goto error; } if (v_o == NULL) { if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { - v_o = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), + _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), (PyDictObject *)BUILTINS(), - name); - if (v_o == NULL) { + name, + v); + if (PyStackRef_IsNull(*v)) { if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobal() returns NULL without raising + /* _PyDict_LoadGlobalStackRef() sets NULL without raising * an exception if the key doesn't exist */ _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, NAME_ERROR_MSG, name); @@ -5328,9 +5328,11 @@ } } } + if (v_o != NULL) { + *v = PyStackRef_FromPyObjectSteal(v_o); + } PyStackRef_CLOSE(mod_or_class_dict); - v = PyStackRef_FromPyObjectSteal(v_o); - stack_pointer[-1] = v; + stack_pointer[-1].bits = (uintptr_t)v; DISPATCH(); } @@ -5341,7 +5343,7 @@ PREDICTED(LOAD_GLOBAL); _Py_CODEUNIT *this_instr = next_instr - 5; (void)this_instr; - _PyStackRef res; + _PyStackRef *res; _PyStackRef null = PyStackRef_NULL; // _SPECIALIZE_LOAD_GLOBAL { @@ -5363,13 +5365,44 @@ /* Skip 1 cache entry */ // _LOAD_GLOBAL { + res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - PyObject *res_o = _PyEval_LoadGlobal(GLOBALS(), BUILTINS(), name); - if (res_o == NULL) goto error; + if (PyDict_CheckExact(GLOBALS()) + && PyDict_CheckExact(BUILTINS())) + { + _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name, + res); + if (PyStackRef_IsNull(*res)) { + if (!_PyErr_Occurred(tstate)) { + /* _PyDict_LoadGlobalStackRef() sets NULL without raising + * an exception if the key doesn't exist */ + _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + } + if (true) goto error; + } + } + else { + PyObject *res_o; + /* Slow-path if globals or builtins is not a dict */ + /* namespace 1: globals */ + if (PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0) goto error; + if (res_o == NULL) { + /* namespace 2: builtins */ + if (PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0) goto error; + if (res_o == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + if (true) goto error; + } + } + *res = PyStackRef_FromPyObjectSteal(res_o); + } null = PyStackRef_NULL; - res = PyStackRef_FromPyObjectSteal(res_o); } - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 866d7d95b580d4..d6ee0598a4b188 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -829,11 +829,13 @@ } case _LOAD_GLOBAL: { - _Py_UopsSymbol *res; + _Py_UopsSymbol **res; _Py_UopsSymbol *null = NULL; - res = sym_new_not_null(ctx); + res = &stack_pointer[0]; + for (int _i = 1; --_i >= 0;) { + res[_i] = sym_new_not_null(ctx); + } null = sym_new_null(ctx); - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); From b213b6f1bd4f1e4a992786a147b3f18e0de69f13 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 18 Aug 2024 21:33:30 +0800 Subject: [PATCH 02/17] fix merge from upstream --- Include/internal/pycore_ceval.h | 2 +- Python/bytecodes.c | 38 +++------------------------------ Python/ceval.c | 20 +++++++++-------- Python/executor_cases.c.h | 36 ++----------------------------- Python/generated_cases.c.h | 38 +++------------------------------ 5 files changed, 20 insertions(+), 114 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index e4af731be0e87f..a97b53028c8f59 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -271,7 +271,7 @@ PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ PyAPI_FUNC(void) _PyObjectArray_Free(PyObject **array, PyObject **scratch); PyAPI_FUNC(PyObject *) _PyEval_GetANext(PyObject *aiter); -PyAPI_FUNC(PyObject *) _PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name); +PyAPI_FUNC(void) _PyEval_LoadGlobalStackRef(PyObject *globals, PyObject *builtins, PyObject *name, _PyStackRef *writeto); PyAPI_FUNC(PyObject *) _PyEval_GetAwaitable(PyObject *iterable, int oparg); PyAPI_FUNC(PyObject *) _PyEval_LoadName(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject *name); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 8fe5ecb4c5bd71..7eaf87d161a9d5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1436,7 +1436,7 @@ dummy_func( locals = PyStackRef_FromPyObjectNew(l); } - inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v: _PyStackRef *)) { + inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v[1])) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); PyObject *v_o = NULL; if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { @@ -1509,40 +1509,8 @@ dummy_func( op(_LOAD_GLOBAL, ( -- res[1], null if (oparg & 1))) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - if (PyDict_CheckExact(GLOBALS()) - && PyDict_CheckExact(BUILTINS())) - { - _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), - (PyDictObject *)BUILTINS(), - name, - res); - if (PyStackRef_IsNull(*res)) { - if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobalStackRef() sets NULL without raising - * an exception if the key doesn't exist */ - _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - } - ERROR_IF(true, error); - } - } - else { - PyObject *res_o; - /* Slow-path if globals or builtins is not a dict */ - /* namespace 1: globals */ - ERROR_IF(PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0, error); - if (res_o == NULL) { - /* namespace 2: builtins */ - ERROR_IF(PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0, error); - if (res_o == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - ERROR_IF(true, error); - } - } - *res = PyStackRef_FromPyObjectSteal(res_o); - } + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + ERROR_IF(PyStackRef_IsNull(*res), error); null = PyStackRef_NULL; } diff --git a/Python/ceval.c b/Python/ceval.c index c685a95b2ef088..9b4cc9553ee621 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3072,15 +3072,14 @@ _PyEval_GetANext(PyObject *aiter) return awaitable; } -PyObject * -_PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name) +void +_PyEval_LoadGlobalStackRef(PyObject *globals, PyObject *builtins, PyObject *name, _PyStackRef *writeto) { - PyObject *res; if (PyDict_CheckExact(globals) && PyDict_CheckExact(builtins)) { - res = _PyDict_LoadGlobal((PyDictObject *)globals, + _PyDict_LoadGlobalStackRef((PyDictObject *)globals, (PyDictObject *)builtins, - name); - if (res == NULL && !PyErr_Occurred()) { + name, writeto); + if (PyStackRef_IsNull(*writeto) && !PyErr_Occurred()) { /* _PyDict_LoadGlobal() returns NULL without raising * an exception if the key doesn't exist */ _PyEval_FormatExcCheckArg(PyThreadState_GET(), PyExc_NameError, @@ -3090,13 +3089,16 @@ _PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name) else { /* Slow-path if globals or builtins is not a dict */ /* namespace 1: globals */ + PyObject *res; if (PyMapping_GetOptionalItem(globals, name, &res) < 0) { - return NULL; + *writeto = PyStackRef_NULL; + return; } if (res == NULL) { /* namespace 2: builtins */ if (PyMapping_GetOptionalItem(builtins, name, &res) < 0) { - return NULL; + *writeto = PyStackRef_NULL; + return; } if (res == NULL) { _PyEval_FormatExcCheckArg( @@ -3104,8 +3106,8 @@ _PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name) NAME_ERROR_MSG, name); } } + *writeto = PyStackRef_FromPyObjectSteal(res); } - return res; } PyObject * diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index f7af64eb2e05ea..2654ea9e0179bd 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1665,40 +1665,8 @@ oparg = CURRENT_OPARG(); res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - if (PyDict_CheckExact(GLOBALS()) - && PyDict_CheckExact(BUILTINS())) - { - _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), - (PyDictObject *)BUILTINS(), - name, - res); - if (PyStackRef_IsNull(*res)) { - if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobalStackRef() sets NULL without raising - * an exception if the key doesn't exist */ - _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - } - if (true) JUMP_TO_ERROR(); - } - } - else { - PyObject *res_o; - /* Slow-path if globals or builtins is not a dict */ - /* namespace 1: globals */ - if (PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0) JUMP_TO_ERROR(); - if (res_o == NULL) { - /* namespace 2: builtins */ - if (PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0) JUMP_TO_ERROR(); - if (res_o == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - if (true) JUMP_TO_ERROR(); - } - } - *res = PyStackRef_FromPyObjectSteal(res_o); - } + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + if (PyStackRef_IsNull(*res)) JUMP_TO_ERROR(); null = PyStackRef_NULL; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f23c19e564069d..9aa9a30160a057 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5289,6 +5289,7 @@ _PyStackRef mod_or_class_dict; _PyStackRef *v; mod_or_class_dict = stack_pointer[-1]; + v = &stack_pointer[-1]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); PyObject *v_o = NULL; if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { @@ -5332,7 +5333,6 @@ *v = PyStackRef_FromPyObjectSteal(v_o); } PyStackRef_CLOSE(mod_or_class_dict); - stack_pointer[-1].bits = (uintptr_t)v; DISPATCH(); } @@ -5367,40 +5367,8 @@ { res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - if (PyDict_CheckExact(GLOBALS()) - && PyDict_CheckExact(BUILTINS())) - { - _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), - (PyDictObject *)BUILTINS(), - name, - res); - if (PyStackRef_IsNull(*res)) { - if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobalStackRef() sets NULL without raising - * an exception if the key doesn't exist */ - _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - } - if (true) goto error; - } - } - else { - PyObject *res_o; - /* Slow-path if globals or builtins is not a dict */ - /* namespace 1: globals */ - if (PyMapping_GetOptionalItem(GLOBALS(), name, &res_o) < 0) goto error; - if (res_o == NULL) { - /* namespace 2: builtins */ - if (PyMapping_GetOptionalItem(BUILTINS(), name, &res_o) < 0) goto error; - if (res_o == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - if (true) goto error; - } - } - *res = PyStackRef_FromPyObjectSteal(res_o); - } + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + if (PyStackRef_IsNull(*res)) goto error; null = PyStackRef_NULL; } if (oparg & 1) stack_pointer[1] = null; From 11d2fb6f6f84d7f5d8ca562f98c271a08f92d4b9 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 18 Aug 2024 22:28:20 +0800 Subject: [PATCH 03/17] use cases generator to generate stack offsets --- Python/bytecodes.c | 14 +++++----- Python/executor_cases.c.h | 9 +++--- Python/generated_cases.c.h | 21 +++++++------- Python/optimizer_cases.c.h | 8 ++---- Tools/cases_generator/generators_common.py | 32 ++++++++++++++++++++-- 5 files changed, 56 insertions(+), 28 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 7eaf87d161a9d5..274c60b1a42f11 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1436,7 +1436,7 @@ dummy_func( locals = PyStackRef_FromPyObjectNew(l); } - inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v[1])) { + inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); PyObject *v_o = NULL; if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { @@ -1449,8 +1449,8 @@ dummy_func( _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), (PyDictObject *)BUILTINS(), name, - v); - if (PyStackRef_IsNull(*v)) { + STACK_ENTRY(v)); + if (PyStackRef_IsNull(v)) { if (!_PyErr_Occurred(tstate)) { /* _PyDict_LoadGlobalStackRef() sets NULL without raising * an exception if the key doesn't exist */ @@ -1477,7 +1477,7 @@ dummy_func( } } if (v_o != NULL) { - *v = PyStackRef_FromPyObjectSteal(v_o); + v = PyStackRef_FromPyObjectSteal(v_o); } DECREF_INPUTS(); } @@ -1507,10 +1507,10 @@ dummy_func( #endif /* ENABLE_SPECIALIZATION */ } - op(_LOAD_GLOBAL, ( -- res[1], null if (oparg & 1))) { + op(_LOAD_GLOBAL, ( -- res, null if (oparg & 1))) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); - ERROR_IF(PyStackRef_IsNull(*res), error); + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, STACK_ENTRY(res)); + ERROR_IF(PyStackRef_IsNull(res), error); null = PyStackRef_NULL; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 2654ea9e0179bd..3586f163d0a723 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1660,14 +1660,15 @@ } case _LOAD_GLOBAL: { - _PyStackRef *res; + _PyStackRef res; _PyStackRef null = PyStackRef_NULL; oparg = CURRENT_OPARG(); - res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); - if (PyStackRef_IsNull(*res)) JUMP_TO_ERROR(); + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &stack_pointer[0]); + res = stack_pointer[0]; + if (PyStackRef_IsNull(res)) JUMP_TO_ERROR(); null = PyStackRef_NULL; + stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 9aa9a30160a057..0a167fc5215376 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5287,9 +5287,8 @@ next_instr += 1; INSTRUCTION_STATS(LOAD_FROM_DICT_OR_GLOBALS); _PyStackRef mod_or_class_dict; - _PyStackRef *v; + _PyStackRef v; mod_or_class_dict = stack_pointer[-1]; - v = &stack_pointer[-1]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); PyObject *v_o = NULL; if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { @@ -5301,9 +5300,9 @@ { _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), (PyDictObject *)BUILTINS(), - name, - v); - if (PyStackRef_IsNull(*v)) { + name, &stack_pointer[0 - 1]); + v = stack_pointer[0 - 1]; + if (PyStackRef_IsNull(v)) { if (!_PyErr_Occurred(tstate)) { /* _PyDict_LoadGlobalStackRef() sets NULL without raising * an exception if the key doesn't exist */ @@ -5330,9 +5329,10 @@ } } if (v_o != NULL) { - *v = PyStackRef_FromPyObjectSteal(v_o); + v = PyStackRef_FromPyObjectSteal(v_o); } PyStackRef_CLOSE(mod_or_class_dict); + stack_pointer[-1] = v; DISPATCH(); } @@ -5343,7 +5343,7 @@ PREDICTED(LOAD_GLOBAL); _Py_CODEUNIT *this_instr = next_instr - 5; (void)this_instr; - _PyStackRef *res; + _PyStackRef res; _PyStackRef null = PyStackRef_NULL; // _SPECIALIZE_LOAD_GLOBAL { @@ -5365,12 +5365,13 @@ /* Skip 1 cache entry */ // _LOAD_GLOBAL { - res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); - if (PyStackRef_IsNull(*res)) goto error; + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &stack_pointer[0]); + res = stack_pointer[0]; + if (PyStackRef_IsNull(res)) goto error; null = PyStackRef_NULL; } + stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index d6ee0598a4b188..866d7d95b580d4 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -829,13 +829,11 @@ } case _LOAD_GLOBAL: { - _Py_UopsSymbol **res; + _Py_UopsSymbol *res; _Py_UopsSymbol *null = NULL; - res = &stack_pointer[0]; - for (int _i = 1; --_i >= 0;) { - res[_i] = sym_new_not_null(ctx); - } + res = sym_new_not_null(ctx); null = sym_new_null(ctx); + stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index dd4057c931ca19..f584a79a139246 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -47,10 +47,10 @@ def write_header( ) -def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str) -> None: +def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, allow_unbalanced_parens=False) -> None: parens = 0 for tkn in tkn_iter: - if tkn.kind == end and parens == 0: + if tkn.kind == end and (parens == 0 or allow_unbalanced_parens): return if tkn.kind == "LPAREN": parens += 1 @@ -77,6 +77,7 @@ def __init__(self, out: CWriter): "DECREF_INPUTS": self.decref_inputs, "SYNC_SP": self.sync_sp, "PyStackRef_FromPyObjectNew": self.py_stack_ref_from_py_object_new, + "STACK_ENTRY": self.stack_entry, } self.out = out @@ -211,6 +212,33 @@ def py_stack_ref_from_py_object_new( # unused portions of the stack to NULL. stack.flush_single_var(self.out, target, uop.stack.outputs) + def stack_entry( + self, + tkn: Token, + tkn_iter: Iterator[Token], + uop: Uop, + stack: Stack, + inst: Instruction | None, + ) -> None: + emit_to(self.out, tkn_iter, "LPAREN") + target = next(tkn_iter) + size = "0" + for output in uop.stack.inputs: + size += f" - {output.size or 1}" + for output in uop.stack.outputs: + if output.name == target.text: + self.out.emit(f" &stack_pointer[{size}]") + break + size += f" + {output.size or 1}" + else: + raise analysis_error("STACK_ENTRY operand is not a stack output", target) + + next(tkn_iter) # Consume ) + emit_to(self.out, tkn_iter, "SEMI", allow_unbalanced_parens=True) + self.emit(";\n") + # Update the variable + self.out.emit(f"{target.text} = stack_pointer[{size}];\n") + def emit_tokens( self, uop: Uop, From ea8d8555406a6b7d726a582ab1a4cf71d1b79940 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 18 Aug 2024 22:46:38 +0800 Subject: [PATCH 04/17] fix bug --- Objects/dictobject.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index b0b8efcaa88b9d..d101ff13887997 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1602,7 +1602,12 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h { PyObject *val; Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); - *value_addr = PyStackRef_FromPyObjectNew(val); + if (val != NULL) { + *value_addr = PyStackRef_FromPyObjectNew(val); + } + else { + *value_addr = PyStackRef_NULL; + } return ix; } From af0c0324fccdc3e4f98d4b98147138f3feb08ee7 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 18 Aug 2024 22:53:06 +0800 Subject: [PATCH 05/17] add type annotations --- Tools/cases_generator/generators_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index f584a79a139246..a89ced34287792 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -47,7 +47,7 @@ def write_header( ) -def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, allow_unbalanced_parens=False) -> None: +def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, allow_unbalanced_parens: bool = False) -> None: parens = 0 for tkn in tkn_iter: if tkn.kind == end and (parens == 0 or allow_unbalanced_parens): From 1a431de167c6f5c8eb0c46388a40c6aa4b7e48c9 Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Mon, 19 Aug 2024 18:14:12 +0800 Subject: [PATCH 06/17] Update Objects/dictobject.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Objects/dictobject.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index d101ff13887997..d428beb3680a2b 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1522,12 +1522,14 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h if (ix >= 0) { if (kind == DICT_KEYS_SPLIT) { PyDictValues *values = _Py_atomic_load_ptr(&mp->ma_values); - if (values == NULL) + if (values == NULL) { goto read_failed; + } uint8_t capacity = _Py_atomic_load_uint8_relaxed(&values->capacity); - if (ix >= (Py_ssize_t)capacity) + if (ix >= (Py_ssize_t)capacity) { goto read_failed; + } *value_addr = PyStackRef_FromPyObjectNew(values->values[ix]); if (PyStackRef_IsNull(*value_addr)) { From 48bebe9afdcbd9728766598a1057833d4a4c2b4d Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Mon, 19 Aug 2024 18:14:22 +0800 Subject: [PATCH 07/17] Update Objects/dictobject.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Objects/dictobject.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index d428beb3680a2b..affcf34071207a 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1535,7 +1535,6 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h if (PyStackRef_IsNull(*value_addr)) { goto read_failed; } - if (values != _Py_atomic_load_ptr(&mp->ma_values)) { goto read_failed; } @@ -1545,7 +1544,6 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h if (PyStackRef_IsNull(*value_addr)) { goto read_failed; } - if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { goto read_failed; } From 6e948b48993479f1781d90465f3424ef79f5995c Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Mon, 19 Aug 2024 18:14:33 +0800 Subject: [PATCH 08/17] Update Objects/dictobject.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Objects/dictobject.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index affcf34071207a..30f05759424721 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1602,12 +1602,7 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h { PyObject *val; Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); - if (val != NULL) { - *value_addr = PyStackRef_FromPyObjectNew(val); - } - else { - *value_addr = PyStackRef_NULL; - } + *value_addr = value == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(value); return ix; } From e23567ae69c026224b85ce7c2f8f7ee14a357004 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Mon, 19 Aug 2024 18:27:13 +0800 Subject: [PATCH 09/17] Address review --- Python/bytecodes.c | 6 +++--- Tools/cases_generator/generators_common.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 274c60b1a42f11..f1f92e9970167a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1447,9 +1447,9 @@ dummy_func( && PyDict_CheckExact(BUILTINS())) { _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), - (PyDictObject *)BUILTINS(), - name, - STACK_ENTRY(v)); + (PyDictObject *)BUILTINS(), + name, + STACK_ENTRY(v)); if (PyStackRef_IsNull(v)) { if (!_PyErr_Occurred(tstate)) { /* _PyDict_LoadGlobalStackRef() sets NULL without raising diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index a89ced34287792..51770a73a982e6 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -47,7 +47,7 @@ def write_header( ) -def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, allow_unbalanced_parens: bool = False) -> None: +def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, *, allow_unbalanced_parens: bool = False) -> None: parens = 0 for tkn in tkn_iter: if tkn.kind == end and (parens == 0 or allow_unbalanced_parens): @@ -233,7 +233,7 @@ def stack_entry( else: raise analysis_error("STACK_ENTRY operand is not a stack output", target) - next(tkn_iter) # Consume ) + next(tkn_iter) # Consume ) emit_to(self.out, tkn_iter, "SEMI", allow_unbalanced_parens=True) self.emit(";\n") # Update the variable From dbdee10958477f2a9588310e48314e913df89afb Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:21:23 +0800 Subject: [PATCH 10/17] partially address review --- Include/internal/pycore_dict.h | 3 ++- Objects/dictobject.c | 27 ++++++++++++++++++++++++++- Python/bytecodes.c | 20 +++++++++----------- Python/generated_cases.c.h | 18 ++++++++---------- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 877b35af1a7169..017246c8282a1d 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -105,7 +105,8 @@ extern Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject extern Py_ssize_t _PyDict_LookupIndex(PyDictObject *, PyObject *); extern Py_ssize_t _PyDictKeys_StringLookup(PyDictKeysObject* dictkeys, PyObject *key); -PyAPI_FUNC(void)_PyDict_LoadGlobalStackRef(PyDictObject *, PyDictObject *, PyObject *, _PyStackRef *); +PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObject *); +PyAPI_FUNC(void) _PyDict_LoadGlobalStackRef(PyDictObject *, PyDictObject *, PyObject *, _PyStackRef *); /* Consumes references to key and value */ PyAPI_FUNC(int) _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value); diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 30f05759424721..0efb6a968dbf80 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2491,8 +2491,33 @@ _PyDict_GetItemStringWithError(PyObject *v, const char *key) * key hash failed, key comparison failed, ...). Return NULL if the key doesn't * exist. Return the value if the key exists. * - * Stores a new stack reference. + * Returns a new reference. */ +PyObject * +_PyDict_LoadGlobal(PyDictObject *globals, PyDictObject *builtins, PyObject *key) +{ + Py_ssize_t ix; + Py_hash_t hash; + PyObject *value; + + hash = _PyObject_HashFast(key); + if (hash == -1) { + return NULL; + } + + /* namespace 1: globals */ + ix = _Py_dict_lookup_threadsafe(globals, key, hash, &value); + if (ix == DKIX_ERROR) + return NULL; + if (ix != DKIX_EMPTY && value != NULL) + return value; + + /* namespace 2: builtins */ + ix = _Py_dict_lookup_threadsafe(builtins, key, hash, &value); + assert(ix >= 0 || value == NULL); + return value; +} + void _PyDict_LoadGlobalStackRef(PyDictObject *globals, PyDictObject *builtins, PyObject *key, _PyStackRef *res) { diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f1f92e9970167a..fb5c8d9acd8ccc 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1438,21 +1438,21 @@ dummy_func( inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - PyObject *v_o = NULL; - if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { + PyObject *v_o; + int err = PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o); + if (err < 0) { ERROR_NO_POP(); } if (v_o == NULL) { if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { - _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), - (PyDictObject *)BUILTINS(), - name, - STACK_ENTRY(v)); - if (PyStackRef_IsNull(v)) { + v_o = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name); + if (v_o == NULL) { if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobalStackRef() sets NULL without raising + /* _PyDict_LoadGlobal() returns NULL without raising * an exception if the key doesn't exist */ _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, NAME_ERROR_MSG, name); @@ -1476,10 +1476,8 @@ dummy_func( } } } - if (v_o != NULL) { - v = PyStackRef_FromPyObjectSteal(v_o); - } DECREF_INPUTS(); + v = PyStackRef_FromPyObjectSteal(v_o); } inst(LOAD_NAME, (-- v)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0a167fc5215376..c909873cb8709f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5290,21 +5290,21 @@ _PyStackRef v; mod_or_class_dict = stack_pointer[-1]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - PyObject *v_o = NULL; - if (PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o) < 0) { + PyObject *v_o; + int err = PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o); + if (err < 0) { goto error; } if (v_o == NULL) { if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { - _PyDict_LoadGlobalStackRef((PyDictObject *)GLOBALS(), + v_o = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), (PyDictObject *)BUILTINS(), - name, &stack_pointer[0 - 1]); - v = stack_pointer[0 - 1]; - if (PyStackRef_IsNull(v)) { + name); + if (v_o == NULL) { if (!_PyErr_Occurred(tstate)) { - /* _PyDict_LoadGlobalStackRef() sets NULL without raising + /* _PyDict_LoadGlobal() returns NULL without raising * an exception if the key doesn't exist */ _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, NAME_ERROR_MSG, name); @@ -5328,10 +5328,8 @@ } } } - if (v_o != NULL) { - v = PyStackRef_FromPyObjectSteal(v_o); - } PyStackRef_CLOSE(mod_or_class_dict); + v = PyStackRef_FromPyObjectSteal(v_o); stack_pointer[-1] = v; DISPATCH(); } From 6d176c011a6daba50535595bad2d7db35ce06501 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:28:12 +0800 Subject: [PATCH 11/17] simplify --- Objects/dictobject.c | 61 ++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 44 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 0efb6a968dbf80..c68fa05394f8ea 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1509,49 +1509,10 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h kind = dk->dk_kind; if (kind != DICT_KEYS_GENERAL) { - if (PyUnicode_CheckExact(key)) { - ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - } - else { - ix = unicodekeys_lookup_generic_threadsafe(mp, dk, key, hash); - } - if (ix == DKIX_KEY_CHANGED) { - goto read_failed; - } - - if (ix >= 0) { - if (kind == DICT_KEYS_SPLIT) { - PyDictValues *values = _Py_atomic_load_ptr(&mp->ma_values); - if (values == NULL) { - goto read_failed; - } - - uint8_t capacity = _Py_atomic_load_uint8_relaxed(&values->capacity); - if (ix >= (Py_ssize_t)capacity) { - goto read_failed; - } - - *value_addr = PyStackRef_FromPyObjectNew(values->values[ix]); - if (PyStackRef_IsNull(*value_addr)) { - goto read_failed; - } - if (values != _Py_atomic_load_ptr(&mp->ma_values)) { - goto read_failed; - } - } - else { - *value_addr = PyStackRef_FromPyObjectNew(DK_UNICODE_ENTRIES(dk)[ix].me_value); - if (PyStackRef_IsNull(*value_addr)) { - goto read_failed; - } - if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { - goto read_failed; - } - } - } - else { - *value_addr = PyStackRef_NULL; - } + PyObject *value; + ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value); + assert (ix >= 0 || value == NULL); + *value_addr = PyStackRef_FromPyObjectSteal(value); } else { ix = dictkeys_generic_lookup_threadsafe(mp, dk, key, hash); @@ -1559,7 +1520,19 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h goto read_failed; } if (ix >= 0) { - *value_addr = PyStackRef_FromPyObjectNew(DK_ENTRIES(dk)[ix].me_value); + PyObject **addr_of_value = &(DK_ENTRIES(dk)[ix].me_value); + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + } + else if (_Py_IsImmortal(value) || + _PyObject_HasDeferredRefcount(value)) { + *value_addr = PyStackRef_FromPyObjectNew(value); + } + else { + *value_addr = PyStackRef_FromPyObjectSteal( + _Py_TryXGetRef(addr_of_value)); + } if (PyStackRef_IsNull(*value_addr)) { goto read_failed; } From 4ff9f177356706a14d8839ee5df4bbc1b8cf0131 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:30:55 +0800 Subject: [PATCH 12/17] fix some stuff --- Objects/dictobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index c68fa05394f8ea..7982f58aebccb8 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1575,7 +1575,7 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h { PyObject *val; Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); - *value_addr = value == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(value); + *value_addr = val == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(val); return ix; } From c644ebffc9af2200ae16eae37400406101bba123 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 28 Aug 2024 20:25:31 +0800 Subject: [PATCH 13/17] specialize for str and split str, move out NULL checks --- Objects/dictobject.c | 95 +++++++++++++++++++++++++++++++++----------- 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 7982f58aebccb8..24c46aea98c12e 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1509,41 +1509,90 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h kind = dk->dk_kind; if (kind != DICT_KEYS_GENERAL) { - PyObject *value; - ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value); - assert (ix >= 0 || value == NULL); - *value_addr = PyStackRef_FromPyObjectSteal(value); - } - else { - ix = dictkeys_generic_lookup_threadsafe(mp, dk, key, hash); + if (PyUnicode_CheckExact(key)) { + ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + } + else { + ix = unicodekeys_lookup_generic_threadsafe(mp, dk, key, hash); + } if (ix == DKIX_KEY_CHANGED) { goto read_failed; } + if (ix >= 0) { - PyObject **addr_of_value = &(DK_ENTRIES(dk)[ix].me_value); - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - } - else if (_Py_IsImmortal(value) || - _PyObject_HasDeferredRefcount(value)) { - *value_addr = PyStackRef_FromPyObjectNew(value); + if (kind == DICT_KEYS_SPLIT) { + PyDictValues *values = _Py_atomic_load_ptr(&mp->ma_values); + if (values == NULL) { + goto read_failed; + } + + uint8_t capacity = _Py_atomic_load_uint8_relaxed(&values->capacity); + if (ix >= (Py_ssize_t)capacity) { + goto read_failed; + } + + PyObject **addr_of_value = &values->values[ix]; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + } + else if (_Py_IsImmortal(value) || + _PyObject_HasDeferredRefcount(value)) { + *value_addr = PyStackRef_FromPyObjectNew(value); + } + else { + PyObject *res = _Py_TryXGetRef(addr_of_value); + if (res == NULL) { + *value_addr = PyStackRef_NULL; + } + else { + *value_addr = PyStackRef_FromPyObjectSteal(res); + } + } + if (PyStackRef_IsNull(*value_addr)) { + goto read_failed; + } + if (values != _Py_atomic_load_ptr(&mp->ma_values)) { + goto read_failed; + } } else { - *value_addr = PyStackRef_FromPyObjectSteal( - _Py_TryXGetRef(addr_of_value)); - } - if (PyStackRef_IsNull(*value_addr)) { - goto read_failed; - } - if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { - goto read_failed; + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + } + else if (_Py_IsImmortal(value) || + _PyObject_HasDeferredRefcount(value)) { + *value_addr = PyStackRef_FromPyObjectNew(value); + } + else { + PyObject *res = _Py_TryXGetRef(addr_of_value); + if (res == NULL) { + *value_addr = PyStackRef_NULL; + } + else { + *value_addr = PyStackRef_FromPyObjectSteal(res); + } + } + if (PyStackRef_IsNull(*value_addr)) { + goto read_failed; + } + if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { + goto read_failed; + } } } else { *value_addr = PyStackRef_NULL; } } + else { + PyObject *value; + ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value); + assert (ix >= 0 || value == NULL); + *value_addr = PyStackRef_FromPyObjectSteal(value); + } return ix; From 64f932ac08fee24c62d46e8bf9fd163b24126d51 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 28 Aug 2024 20:28:36 +0800 Subject: [PATCH 14/17] remove allow_unbalanced_parens --- Tools/cases_generator/generators_common.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 51770a73a982e6..4b8a769589c732 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -47,10 +47,10 @@ def write_header( ) -def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str, *, allow_unbalanced_parens: bool = False) -> None: +def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str) -> None: parens = 0 for tkn in tkn_iter: - if tkn.kind == end and (parens == 0 or allow_unbalanced_parens): + if tkn.kind == end and parens == 0: return if tkn.kind == "LPAREN": parens += 1 @@ -234,8 +234,12 @@ def stack_entry( raise analysis_error("STACK_ENTRY operand is not a stack output", target) next(tkn_iter) # Consume ) - emit_to(self.out, tkn_iter, "SEMI", allow_unbalanced_parens=True) - self.emit(";\n") + # Emit all the way to SEMI + for tkn in tkn_iter: + self.out.emit(tkn) + if tkn.kind == "SEMI": + break + self.emit("\n") # Update the variable self.out.emit(f"{target.text} = stack_pointer[{size}];\n") From bfd4400299f21b34b9f68d8d80cd9d5e8e5591e3 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 28 Aug 2024 20:30:12 +0800 Subject: [PATCH 15/17] Add a & for STACK_ENTRY --- Python/bytecodes.c | 2 +- Tools/cases_generator/generators_common.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index fb5c8d9acd8ccc..23fb96bc264790 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1507,7 +1507,7 @@ dummy_func( op(_LOAD_GLOBAL, ( -- res, null if (oparg & 1))) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, STACK_ENTRY(res)); + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &STACK_ENTRY(res)); ERROR_IF(PyStackRef_IsNull(res), error); null = PyStackRef_NULL; } diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 4b8a769589c732..e710305a7cc15c 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -227,7 +227,7 @@ def stack_entry( size += f" - {output.size or 1}" for output in uop.stack.outputs: if output.name == target.text: - self.out.emit(f" &stack_pointer[{size}]") + self.out.emit(f"stack_pointer[{size}]") break size += f" + {output.size or 1}" else: From 0c1ee7e42c71f689dd97392f1abefe44e4db8c0d Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 8 Sep 2024 16:07:30 +0800 Subject: [PATCH 16/17] Address review Co-Authored-By: Sam Gross <655866+colesbury@users.noreply.github.com> --- Objects/dictobject.c | 127 +++++++++---------------------------------- 1 file changed, 27 insertions(+), 100 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 24c46aea98c12e..4a9551f819a0da 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1499,113 +1499,39 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { - PyDictKeysObject *dk; - DictKeysKind kind; - Py_ssize_t ix; - - ensure_shared_on_read(mp); - - dk = _Py_atomic_load_ptr(&mp->ma_keys); - kind = dk->dk_kind; - - if (kind != DICT_KEYS_GENERAL) { - if (PyUnicode_CheckExact(key)) { - ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - } - else { - ix = unicodekeys_lookup_generic_threadsafe(mp, dk, key, hash); - } - if (ix == DKIX_KEY_CHANGED) { - goto read_failed; + PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { + Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + if (ix == DKIX_EMPTY) { + *value_addr = PyStackRef_NULL; + return ix; } - - if (ix >= 0) { - if (kind == DICT_KEYS_SPLIT) { - PyDictValues *values = _Py_atomic_load_ptr(&mp->ma_values); - if (values == NULL) { - goto read_failed; - } - - uint8_t capacity = _Py_atomic_load_uint8_relaxed(&values->capacity); - if (ix >= (Py_ssize_t)capacity) { - goto read_failed; - } - - PyObject **addr_of_value = &values->values[ix]; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - } - else if (_Py_IsImmortal(value) || - _PyObject_HasDeferredRefcount(value)) { - *value_addr = PyStackRef_FromPyObjectNew(value); - } - else { - PyObject *res = _Py_TryXGetRef(addr_of_value); - if (res == NULL) { - *value_addr = PyStackRef_NULL; - } - else { - *value_addr = PyStackRef_FromPyObjectSteal(res); - } - } - if (PyStackRef_IsNull(*value_addr)) { - goto read_failed; - } - if (values != _Py_atomic_load_ptr(&mp->ma_values)) { - goto read_failed; - } + else if (ix >= 0) { + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; } - else { - PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - } - else if (_Py_IsImmortal(value) || - _PyObject_HasDeferredRefcount(value)) { - *value_addr = PyStackRef_FromPyObjectNew(value); - } - else { - PyObject *res = _Py_TryXGetRef(addr_of_value); - if (res == NULL) { - *value_addr = PyStackRef_NULL; - } - else { - *value_addr = PyStackRef_FromPyObjectSteal(res); - } - } - if (PyStackRef_IsNull(*value_addr)) { - goto read_failed; - } - if (dk != _Py_atomic_load_ptr(&mp->ma_keys)) { - goto read_failed; - } + if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return ix; + } + if (_Py_TryIncrefCompare(addr_of_value, value)) { + *value_addr = PyStackRef_FromPyObjectSteal(value); + return ix; } } - else { - *value_addr = PyStackRef_NULL; - } + } + + PyObject *obj; + Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); + if (ix >= 0 && obj != NULL) { + *value_addr = PyStackRef_FromPyObjectSteal(obj); } else { - PyObject *value; - ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value); - assert (ix >= 0 || value == NULL); - *value_addr = PyStackRef_FromPyObjectSteal(value); + *value_addr = PyStackRef_NULL; } - - return ix; - - PyObject *value; -read_failed: - // In addition to the normal races of the dict being modified the _Py_TryXGetRef - // can all fail if they don't yet have a shared ref count. That can happen here - // or in the *_lookup_* helper. In that case we need to take the lock to avoid - // mutation and do a normal incref which will make them shared. - Py_BEGIN_CRITICAL_SECTION(mp); - ix = _Py_dict_lookup(mp, key, hash, &value); - *value_addr = value == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(value); - Py_END_CRITICAL_SECTION(); return ix; } @@ -2549,6 +2475,7 @@ _PyDict_LoadGlobalStackRef(PyDictObject *globals, PyDictObject *builtins, PyObje hash = _PyObject_HashFast(key); if (hash == -1) { *res = PyStackRef_NULL; + return; } /* namespace 1: globals */ From ad33dd1c40a46942aeacf0bc8e4f40e745a009e0 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 12 Sep 2024 20:20:53 +0800 Subject: [PATCH 17/17] Change to use array instead of STACK_ENTRY --- Python/bytecodes.c | 7 +++-- Python/executor_cases.c.h | 9 +++--- Python/generated_cases.c.h | 9 +++--- Python/optimizer_cases.c.h | 8 ++++-- Tools/cases_generator/generators_common.py | 32 ---------------------- 5 files changed, 17 insertions(+), 48 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 23fb96bc264790..ea7e335d88c2a6 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1505,10 +1505,11 @@ dummy_func( #endif /* ENABLE_SPECIALIZATION */ } - op(_LOAD_GLOBAL, ( -- res, null if (oparg & 1))) { + // res[1] because we need a pointer to res to pass it to _PyEval_LoadGlobalStackRef + op(_LOAD_GLOBAL, ( -- res[1], null if (oparg & 1))) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &STACK_ENTRY(res)); - ERROR_IF(PyStackRef_IsNull(res), error); + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + ERROR_IF(PyStackRef_IsNull(*res), error); null = PyStackRef_NULL; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 3586f163d0a723..2654ea9e0179bd 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1660,15 +1660,14 @@ } case _LOAD_GLOBAL: { - _PyStackRef res; + _PyStackRef *res; _PyStackRef null = PyStackRef_NULL; oparg = CURRENT_OPARG(); + res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &stack_pointer[0]); - res = stack_pointer[0]; - if (PyStackRef_IsNull(res)) JUMP_TO_ERROR(); + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + if (PyStackRef_IsNull(*res)) JUMP_TO_ERROR(); null = PyStackRef_NULL; - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index c909873cb8709f..b29b11266675a7 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5341,7 +5341,7 @@ PREDICTED(LOAD_GLOBAL); _Py_CODEUNIT *this_instr = next_instr - 5; (void)this_instr; - _PyStackRef res; + _PyStackRef *res; _PyStackRef null = PyStackRef_NULL; // _SPECIALIZE_LOAD_GLOBAL { @@ -5363,13 +5363,12 @@ /* Skip 1 cache entry */ // _LOAD_GLOBAL { + res = &stack_pointer[0]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1); - _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &stack_pointer[0]); - res = stack_pointer[0]; - if (PyStackRef_IsNull(res)) goto error; + _PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, res); + if (PyStackRef_IsNull(*res)) goto error; null = PyStackRef_NULL; } - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 866d7d95b580d4..d6ee0598a4b188 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -829,11 +829,13 @@ } case _LOAD_GLOBAL: { - _Py_UopsSymbol *res; + _Py_UopsSymbol **res; _Py_UopsSymbol *null = NULL; - res = sym_new_not_null(ctx); + res = &stack_pointer[0]; + for (int _i = 1; --_i >= 0;) { + res[_i] = sym_new_not_null(ctx); + } null = sym_new_null(ctx); - stack_pointer[0] = res; if (oparg & 1) stack_pointer[1] = null; stack_pointer += 1 + (oparg & 1); assert(WITHIN_STACK_BOUNDS()); diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index e710305a7cc15c..dd4057c931ca19 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -77,7 +77,6 @@ def __init__(self, out: CWriter): "DECREF_INPUTS": self.decref_inputs, "SYNC_SP": self.sync_sp, "PyStackRef_FromPyObjectNew": self.py_stack_ref_from_py_object_new, - "STACK_ENTRY": self.stack_entry, } self.out = out @@ -212,37 +211,6 @@ def py_stack_ref_from_py_object_new( # unused portions of the stack to NULL. stack.flush_single_var(self.out, target, uop.stack.outputs) - def stack_entry( - self, - tkn: Token, - tkn_iter: Iterator[Token], - uop: Uop, - stack: Stack, - inst: Instruction | None, - ) -> None: - emit_to(self.out, tkn_iter, "LPAREN") - target = next(tkn_iter) - size = "0" - for output in uop.stack.inputs: - size += f" - {output.size or 1}" - for output in uop.stack.outputs: - if output.name == target.text: - self.out.emit(f"stack_pointer[{size}]") - break - size += f" + {output.size or 1}" - else: - raise analysis_error("STACK_ENTRY operand is not a stack output", target) - - next(tkn_iter) # Consume ) - # Emit all the way to SEMI - for tkn in tkn_iter: - self.out.emit(tkn) - if tkn.kind == "SEMI": - break - self.emit("\n") - # Update the variable - self.out.emit(f"{target.text} = stack_pointer[{size}];\n") - def emit_tokens( self, uop: Uop,