From 23776cf89a76a54e73a17a422e47293c6299c006 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 | 22 ++++++++++++++++++++++ Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_metadata.h | 2 +- Python/bytecodes.c | 7 +++---- Python/executor_cases.c.h | 9 ++++----- Python/generated_cases.c.h | 9 ++++----- Python/optimizer_cases.c.h | 8 +++++--- 7 files changed, 40 insertions(+), 19 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 27f67d57b2fb794..0867bc1c8310b44 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,27 @@ PyCell_GetRef(PyCellObject *cell) return res; } +static inline void +_PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) +{ + PyObject *value = _Py_TryXGetRef(&cell->ob_ref); + if (value != NULL) { + if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return; + } + *value_addr = PyStackRef_FromPyObjectSteal(value); + return; + } + // fallback + 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 51479afae3833d8..45918dc22f541d2 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 4d0ab22e6aa8f39..8a70b836c1b6333 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 5f194aec0073c81..2487e2277f408f0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1647,14 +1647,13 @@ dummy_func( value = PyStackRef_FromPyObjectSteal(value_o); } - inst(LOAD_DEREF, ( -- value)) { + inst(LOAD_DEREF, ( -- value[1])) { 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 7285acec0bacaf6..1a15012dbdc1f50 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1821,16 +1821,15 @@ } case _LOAD_DEREF: { - _PyStackRef value; + _PyStackRef *value; oparg = CURRENT_OPARG(); + value = &stack_pointer[0]; 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()); break; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 58792a2101ab28f..d03fcb8bc071a6c 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5500,15 +5500,14 @@ frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(LOAD_DEREF); - _PyStackRef value; + _PyStackRef *value; + value = &stack_pointer[0]; 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()); DISPATCH(); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index a6cfa271ae6758c..5caa38822f1f553 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -894,9 +894,11 @@ } case _LOAD_DEREF: { - _Py_UopsSymbol *value; - value = sym_new_not_null(ctx); - stack_pointer[0] = value; + _Py_UopsSymbol **value; + value = &stack_pointer[0]; + for (int _i = 1; --_i >= 0;) { + value[_i] = sym_new_not_null(ctx); + } stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break;