From d9e018cd0ec95e74c9d3703c2dd5422e648b882d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 24 Sep 2024 12:14:46 -0700 Subject: [PATCH] gh-123358: Update LOAD_DEREF to use stackref and atomic incref --- Include/internal/pycore_cell.h | 26 +++++++++++++++++++++++ Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_metadata.h | 2 +- Python/bytecodes.c | 5 ++--- Python/executor_cases.c.h | 5 ++--- Python/generated_cases.c.h | 5 ++--- 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 27f67d57b2fb79..ff9ec82d23f39e 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -2,6 +2,7 @@ #define Py_INTERNAL_CELL_H #include "pycore_critical_section.h" +#include "pycore_object.h" #ifdef __cplusplus extern "C" { @@ -42,6 +43,31 @@ PyCell_GetRef(PyCellObject *cell) return res; } +static inline void +_PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) +{ + PyObject *value; +#ifdef Py_GIL_DISABLED + value = _Py_atomic_load_ptr(&cell->ob_ref); + if (value != NULL) { + if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return; + } + if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { + *value_addr = _PyStackRef_FromPyObjectSteal(value); + return; + } + } +#endif + value = PyCell_GetRef(cell); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + return; + } + *value_addr = PyStackRef_FromPyObjectSteal(value); +} + #ifdef __cplusplus } #endif diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 51479afae3833d..45918dc22f541d 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1144,7 +1144,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[264] = { [LOAD_BUILD_CLASS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [LOAD_COMMON_CONSTANT] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [LOAD_CONST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_CONST_FLAG | HAS_PURE_FLAG }, - [LOAD_DEREF] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, + [LOAD_DEREF] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [LOAD_FAST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_PURE_FLAG }, [LOAD_FAST_AND_CLEAR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [LOAD_FAST_CHECK] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 4d0ab22e6aa8f3..8a70b836c1b633 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -123,7 +123,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_MAKE_CELL] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, [_DELETE_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_LOAD_FROM_DICT_OR_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, - [_LOAD_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, + [_LOAD_DEREF] = HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_STORE_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ESCAPES_FLAG, [_COPY_FREE_VARS] = HAS_ARG_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 5f194aec0073c8..7a1bfb2e8f577a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1649,12 +1649,11 @@ dummy_func( inst(LOAD_DEREF, ( -- value)) { PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); ERROR_IF(true, error); } - value = PyStackRef_FromPyObjectSteal(value_o); } inst(STORE_DEREF, (v --)) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 7285acec0bacaf..3ccac5a16cbf6c 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1824,12 +1824,11 @@ _PyStackRef value; oparg = CURRENT_OPARG(); PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) JUMP_TO_ERROR(); } - value = PyStackRef_FromPyObjectSteal(value_o); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 58792a2101ab28..eac658b97423eb 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5502,12 +5502,11 @@ INSTRUCTION_STATS(LOAD_DEREF); _PyStackRef value; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) goto error; } - value = PyStackRef_FromPyObjectSteal(value_o); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS());