From 34e6b9f7ce6c0f793484da38328e1e353b6810f9 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 28 Jun 2024 22:19:01 +0800 Subject: [PATCH 01/25] Fix a few wrong steals in bytecodes.c --- Python/bytecodes.c | 16 ++++++++-------- Python/ceval.c | 2 +- Python/executor_cases.c.h | 32 ++++++++++++++++---------------- Python/generated_cases.c.h | 16 ++++++++-------- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 8dfce77dfca297..50978a0dc87694 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -230,7 +230,7 @@ dummy_func( } replicate(8) pure inst(LOAD_FAST, (-- value)) { - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); } @@ -673,7 +673,7 @@ dummy_func( err = 1; } else { - err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); Py_DECREF(slice); } PyStackRef_CLOSE(v); @@ -789,7 +789,7 @@ dummy_func( inst(SET_ADD, (set, unused[oparg-1], v -- set, unused[oparg-1])) { int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set), - PyStackRef_AsPyObjectSteal(v)); + PyStackRef_AsPyObjectBorrow(v)); DECREF_INPUTS(); ERROR_IF(err, error); } @@ -813,7 +813,7 @@ dummy_func( op(_STORE_SUBSCR, (v, container, sub -- )) { /* container[sub] = v */ - int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(v)); + int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectBorrow(sub), PyStackRef_AsPyObjectBorrow(v)); DECREF_INPUTS(); ERROR_IF(err, error); } @@ -1235,7 +1235,7 @@ dummy_func( inst(POP_EXCEPT, (exc_value -- )) { _PyErr_StackItem *exc_info = tstate->exc_info; Py_XSETREF(exc_info->exc_value, - PyStackRef_AsPyObjectBorrow(exc_value) == Py_None + PyStackRef_Is(exc_value, PyStackRef_None) ? NULL : PyStackRef_AsPyObjectSteal(exc_value)); } @@ -1330,9 +1330,9 @@ dummy_func( ERROR_IF(true, error); } if (PyDict_CheckExact(ns)) - err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); else - err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); DECREF_INPUTS(); ERROR_IF(err, error); } @@ -1450,7 +1450,7 @@ dummy_func( op(_STORE_ATTR, (v, owner --)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); int err = PyObject_SetAttr(PyStackRef_AsPyObjectBorrow(owner), - name, PyStackRef_AsPyObjectSteal(v)); + name, PyStackRef_AsPyObjectBorrow(v)); DECREF_INPUTS(); ERROR_IF(err, error); } diff --git a/Python/ceval.c b/Python/ceval.c index 2412256ace8c8d..f4b3a417025c14 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1619,7 +1619,7 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func, goto kw_fail; } - if (PyDict_SetItem(kwdict, keyword, PyStackRef_AsPyObjectSteal(value_stackref)) == -1) { + if (PyDict_SetItem(kwdict, keyword, PyStackRef_AsPyObjectBorrow(value_stackref)) == -1) { goto kw_fail; } PyStackRef_CLOSE(value_stackref); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 38437c6f2c087c..76b7a9b4b15ae9 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -54,7 +54,7 @@ _PyStackRef value; oparg = 0; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -66,7 +66,7 @@ _PyStackRef value; oparg = 1; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -78,7 +78,7 @@ _PyStackRef value; oparg = 2; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -90,7 +90,7 @@ _PyStackRef value; oparg = 3; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -102,7 +102,7 @@ _PyStackRef value; oparg = 4; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -114,7 +114,7 @@ _PyStackRef value; oparg = 5; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -126,7 +126,7 @@ _PyStackRef value; oparg = 6; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -138,7 +138,7 @@ _PyStackRef value; oparg = 7; assert(oparg == CURRENT_OPARG()); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -149,7 +149,7 @@ case _LOAD_FAST: { _PyStackRef value; oparg = CURRENT_OPARG(); - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -757,7 +757,7 @@ err = 1; } else { - err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); Py_DECREF(slice); } PyStackRef_CLOSE(v); @@ -939,7 +939,7 @@ v = stack_pointer[-1]; set = stack_pointer[-2 - (oparg-1)]; int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set), - PyStackRef_AsPyObjectSteal(v)); + PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); if (err) JUMP_TO_ERROR(); stack_pointer += -1; @@ -955,7 +955,7 @@ container = stack_pointer[-2]; v = stack_pointer[-3]; /* container[sub] = v */ - int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(v)); + int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectBorrow(sub), PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); PyStackRef_CLOSE(container); PyStackRef_CLOSE(sub); @@ -1281,7 +1281,7 @@ exc_value = stack_pointer[-1]; _PyErr_StackItem *exc_info = tstate->exc_info; Py_XSETREF(exc_info->exc_value, - PyStackRef_AsPyObjectBorrow(exc_value) == Py_None + PyStackRef_Is(exc_value, PyStackRef_None) ? NULL : PyStackRef_AsPyObjectSteal(exc_value)); stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); @@ -1338,9 +1338,9 @@ if (true) JUMP_TO_ERROR(); } if (PyDict_CheckExact(ns)) - err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); else - err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); if (err) JUMP_TO_ERROR(); stack_pointer += -1; @@ -1483,7 +1483,7 @@ v = stack_pointer[-2]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); int err = PyObject_SetAttr(PyStackRef_AsPyObjectBorrow(owner), - name, PyStackRef_AsPyObjectSteal(v)); + name, PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); PyStackRef_CLOSE(owner); if (err) JUMP_TO_ERROR(); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 14959fb31be201..32b22aff14a768 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4667,7 +4667,7 @@ next_instr += 1; INSTRUCTION_STATS(LOAD_FAST); _PyStackRef value; - assert(PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)) != NULL); + assert(!PyStackRef_IsNull(GETLOCAL(oparg))); value = PyStackRef_DUP(GETLOCAL(oparg)); stack_pointer[0] = value; stack_pointer += 1; @@ -5359,7 +5359,7 @@ exc_value = stack_pointer[-1]; _PyErr_StackItem *exc_info = tstate->exc_info; Py_XSETREF(exc_info->exc_value, - PyStackRef_AsPyObjectBorrow(exc_value) == Py_None + PyStackRef_Is(exc_value, PyStackRef_None) ? NULL : PyStackRef_AsPyObjectSteal(exc_value)); stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); @@ -5876,7 +5876,7 @@ v = stack_pointer[-1]; set = stack_pointer[-2 - (oparg-1)]; int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set), - PyStackRef_AsPyObjectSteal(v)); + PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); if (err) goto pop_1_error; stack_pointer += -1; @@ -5977,7 +5977,7 @@ { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); int err = PyObject_SetAttr(PyStackRef_AsPyObjectBorrow(owner), - name, PyStackRef_AsPyObjectSteal(v)); + name, PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); PyStackRef_CLOSE(owner); if (err) goto pop_2_error; @@ -6215,9 +6215,9 @@ if (true) goto pop_1_error; } if (PyDict_CheckExact(ns)) - err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyDict_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); else - err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(ns, name, PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); if (err) goto pop_1_error; stack_pointer += -1; @@ -6244,7 +6244,7 @@ err = 1; } else { - err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectSteal(v)); + err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); Py_DECREF(slice); } PyStackRef_CLOSE(v); @@ -6285,7 +6285,7 @@ v = stack_pointer[-3]; { /* container[sub] = v */ - int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(v)); + int err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), PyStackRef_AsPyObjectBorrow(sub), PyStackRef_AsPyObjectBorrow(v)); PyStackRef_CLOSE(v); PyStackRef_CLOSE(container); PyStackRef_CLOSE(sub); From b3b6851c6bf2ba7fcd124c6548152bfd166cddf3 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sat, 29 Jun 2024 01:48:46 +0800 Subject: [PATCH 02/25] Deferred refcount GC --- Include/internal/pycore_frame.h | 24 +++++++++++++ Include/internal/pycore_gc.h | 3 ++ Include/internal/pycore_stackref.h | 6 ++-- Objects/object.c | 7 ---- Python/bytecodes.c | 19 +++++----- Python/executor_cases.c.h | 37 +++++++++---------- Python/frame.c | 6 ++++ Python/gc_free_threading.c | 57 ++++++++++++++++++++++++++---- Python/generated_cases.c.h | 31 ++++++++-------- Python/optimizer_cases.c.h | 16 +++++---- Python/pystate.c | 7 ---- 11 files changed, 142 insertions(+), 71 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 1e0368faa5b510..94f300d8d97555 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -121,6 +121,13 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame * // Don't leave a dangling pointer to the old frame when creating generators // and coroutines: dest->previous = NULL; + +#ifdef Py_GIL_DISABLED + PyCodeObject *co = (PyCodeObject *)dest->f_executable; + for (int i = src->stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) { + dest->localsplus[i] = PyStackRef_NULL; + } +#endif } /* Consumes reference to func and locals. @@ -146,6 +153,16 @@ _PyFrame_Initialize( for (int i = null_locals_from; i < code->co_nlocalsplus; i++) { frame->localsplus[i] = PyStackRef_NULL; } + +#ifdef Py_GIL_DISABLED + // On GIL disabled, we walk the entire stack in GC. Since stacktop + // is not always in sync with the real stack pointer, we have + // no choice but to traverse the entire stack. + // This just makes sure we don't pass the GC invalid stack values. + for (int i = code->co_nlocalsplus; i < code->co_nlocalsplus + code->co_stacksize; i++) { + frame->localsplus[i] = PyStackRef_NULL; + } +#endif } /* Gets the pointer to the locals array @@ -305,6 +322,13 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int frame->instr_ptr = _PyCode_CODE(code); frame->owner = FRAME_OWNED_BY_THREAD; frame->return_offset = 0; + +#ifdef Py_GIL_DISABLED + assert(code->co_nlocalsplus == 0); + for (int i = 0; i < code->co_stacksize; i++) { + frame->localsplus[i] = PyStackRef_NULL; + } +#endif return frame; } diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index 28e34d3809634c..d2460a8f083562 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -388,6 +388,9 @@ extern void _Py_RunGC(PyThreadState *tstate); extern void _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp); #endif +extern int _Py_visit_decref(PyObject *op, void *arg); +extern int _Py_visit_decref_unreachable(PyObject *op, void *data); + #ifdef __cplusplus } #endif diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 32e445dd96f9a1..e28a06fbca14f9 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -163,8 +163,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj) // Make sure we don't take an already tagged value. assert(((uintptr_t)obj & Py_TAG_BITS) == 0); assert(obj != NULL); - // TODO (gh-117139): Add deferred objects later. - if (_Py_IsImmortal(obj)) { + if (_Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj)) { return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; } else { @@ -236,7 +235,8 @@ PyStackRef_DUP(_PyStackRef stackref) #ifdef Py_GIL_DISABLED if (PyStackRef_IsDeferred(stackref)) { assert(PyStackRef_IsNull(stackref) || - _Py_IsImmortal(PyStackRef_AsPyObjectBorrow(stackref))); + _Py_IsImmortal(PyStackRef_AsPyObjectBorrow(stackref)) || + _PyObject_HasDeferredRefcount(PyStackRef_AsPyObjectBorrow(stackref))); return stackref; } Py_INCREF(PyStackRef_AsPyObjectBorrow(stackref)); diff --git a/Objects/object.c b/Objects/object.c index c4622359bb1035..973e2ebbfa78f4 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2428,13 +2428,6 @@ _PyObject_SetDeferredRefcount(PyObject *op) assert(_Py_IsOwnedByCurrentThread(op)); assert(op->ob_ref_shared == 0); _PyObject_SET_GC_BITS(op, _PyGC_BITS_DEFERRED); - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (_Py_atomic_load_int_relaxed(&interp->gc.immortalize) == 1) { - // gh-117696: immortalize objects instead of using deferred reference - // counting for now. - _Py_SetImmortal(op); - return; - } op->ob_ref_local += 1; op->ob_ref_shared = _Py_REF_QUEUED; #endif diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 50978a0dc87694..07b8c9b365717b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1277,13 +1277,13 @@ dummy_func( } } - tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value)) { + tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value: _PyStackRef *)) { PyObject *exc_value = PyStackRef_AsPyObjectBorrow(exc_value_st); assert(throwflag); assert(exc_value && PyExceptionInstance_Check(exc_value)); if (PyErr_GivenExceptionMatches(exc_value, PyExc_StopIteration)) { - value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + *value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); DECREF_INPUTS(); none = PyStackRef_None; } @@ -1385,14 +1385,14 @@ dummy_func( macro(UNPACK_SEQUENCE) = _SPECIALIZE_UNPACK_SEQUENCE + _UNPACK_SEQUENCE; - inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1, val0)) { + inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1: _PyStackRef *, val0: _PyStackRef *)) { assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); DEOPT_IF(!PyTuple_CheckExact(seq_o)); DEOPT_IF(PyTuple_GET_SIZE(seq_o) != 2); STAT_INC(UNPACK_SEQUENCE, hit); - val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); DECREF_INPUTS(); } @@ -2229,7 +2229,7 @@ dummy_func( _LOAD_ATTR_WITH_HINT + unused/5; - split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr, null if (oparg & 1))) { + split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr: _PyStackRef *, null if (oparg & 1))) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2237,7 +2237,7 @@ dummy_func( DEOPT_IF(attr_o == NULL); STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - attr = PyStackRef_FromPyObjectNew(attr_o); + *attr = PyStackRef_FromPyObjectNew(attr_o); DECREF_INPUTS(); } @@ -2256,10 +2256,10 @@ dummy_func( } - split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr, null if (oparg & 1))) { + split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr: _PyStackRef *, null if (oparg & 1))) { STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - attr = PyStackRef_FromPyObjectNew(descr); + *attr = PyStackRef_FromPyObjectNew(descr); null = PyStackRef_NULL; DECREF_INPUTS(); } @@ -3484,6 +3484,7 @@ dummy_func( self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); stack_pointer[-1 - oparg] = self; // Patch stack as it is used by _PY_FRAME_GENERAL method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 76b7a9b4b15ae9..b14823b87eed96 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1384,10 +1384,12 @@ case _UNPACK_SEQUENCE_TWO_TUPLE: { _PyStackRef seq; - _PyStackRef val1; - _PyStackRef val0; + _PyStackRef *val1; + _PyStackRef *val0; oparg = CURRENT_OPARG(); seq = stack_pointer[-1]; + val1 = &stack_pointer[-1]; + val0 = &stack_pointer[0]; assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); if (!PyTuple_CheckExact(seq_o)) { @@ -1399,11 +1401,9 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(UNPACK_SEQUENCE, hit); - val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); PyStackRef_CLOSE(seq); - stack_pointer[-1] = val1; - stack_pointer[0] = val0; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; @@ -2364,10 +2364,11 @@ case _LOAD_ATTR_SLOT_0: { _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; + attr = &stack_pointer[-1]; uint16_t index = (uint16_t)CURRENT_OPERAND(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2378,18 +2379,18 @@ } STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - attr = PyStackRef_FromPyObjectNew(attr_o); + *attr = PyStackRef_FromPyObjectNew(attr_o); PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; break; } case _LOAD_ATTR_SLOT_1: { _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; + attr = &stack_pointer[-1]; uint16_t index = (uint16_t)CURRENT_OPERAND(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2400,9 +2401,8 @@ } STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - attr = PyStackRef_FromPyObjectNew(attr_o); + *attr = PyStackRef_FromPyObjectNew(attr_o); PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -2430,33 +2430,33 @@ case _LOAD_ATTR_CLASS_0: { _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; + attr = &stack_pointer[-1]; PyObject *descr = (PyObject *)CURRENT_OPERAND(); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - attr = PyStackRef_FromPyObjectNew(descr); + *attr = PyStackRef_FromPyObjectNew(descr); null = PyStackRef_NULL; PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; break; } case _LOAD_ATTR_CLASS_1: { _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; + attr = &stack_pointer[-1]; PyObject *descr = (PyObject *)CURRENT_OPERAND(); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - attr = PyStackRef_FromPyObjectNew(descr); + *attr = PyStackRef_FromPyObjectNew(descr); null = PyStackRef_NULL; PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3534,6 +3534,7 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); stack_pointer[-1 - oparg] = self; // Patch stack as it is used by _PY_FRAME_GENERAL method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); stack_pointer[-2 - oparg] = method; diff --git a/Python/frame.c b/Python/frame.c index 9c7e59601e6faf..19f99b50d94b83 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -20,6 +20,12 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg) int i = 0; /* locals and stack */ for (; i stacktop; i++) { +#ifdef Py_GIL_DISABLED + if (PyStackRef_IsDeferred(locals[i]) && + (visit == _Py_visit_decref || visit == _Py_visit_decref_unreachable)) { + continue; + } +#endif Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); } return 0; diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index f19362c9573812..b1391267889ed7 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -302,6 +302,37 @@ gc_visit_heaps(PyInterpreterState *interp, mi_block_visit_fun *visitor, return err; } +static inline void +gc_visit_stackref(_PyStackRef stackref) +{ + // Note: we MUST check that it is deferred before checking the rest. + // Otherwise we might read into invalid memory due to non-deferred references + // being dead already. + if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNull(stackref)) { + PyObject *curr_o = PyStackRef_AsPyObjectBorrow(stackref); + if (!_Py_IsImmortal(curr_o)) { + gc_add_refs(curr_o, 1); + } + } +} + +static void +gc_visit_thread_stacks(PyInterpreterState *interp) +{ + HEAD_LOCK(&_PyRuntime); + for (PyThreadState *p = interp->threads.head; p != NULL; p = p->next) { + _PyInterpreterFrame *curr_frame = p->current_frame; + while (curr_frame != NULL) { + PyCodeObject *co = (PyCodeObject *)curr_frame->f_executable; + for (int i = 0; i < co->co_nlocalsplus + co->co_stacksize; i++) { + gc_visit_stackref(curr_frame->localsplus[i]); + } + curr_frame = curr_frame->previous; + } + } + HEAD_UNLOCK(&_PyRuntime); +} + static void merge_queued_objects(_PyThreadStateImpl *tstate, struct collection_state *state) { @@ -355,8 +386,8 @@ process_delayed_frees(PyInterpreterState *interp) } // Subtract an incoming reference from the computed "gc_refs" refcount. -static int -visit_decref(PyObject *op, void *arg) +int +_Py_visit_decref(PyObject *op, void *arg) { if (_PyObject_GC_IS_TRACKED(op) && !_Py_IsImmortal(op)) { // If update_refs hasn't reached this object yet, mark it @@ -423,7 +454,7 @@ update_refs(const mi_heap_t *heap, const mi_heap_area_t *area, // Subtract internal references from ob_tid. Objects with ob_tid > 0 // are directly reachable from outside containers, and so can't be // collected. - Py_TYPE(op)->tp_traverse(op, visit_decref, NULL); + Py_TYPE(op)->tp_traverse(op, _Py_visit_decref, NULL); return true; } @@ -554,6 +585,8 @@ deduce_unreachable_heap(PyInterpreterState *interp, // incoming references. gc_visit_heaps(interp, &update_refs, &state->base); + gc_visit_thread_stacks(interp); + #ifdef GC_DEBUG // Check that all objects are marked as unreachable and that the computed // reference count difference (stored in `ob_tid`) is non-negative. @@ -613,6 +646,18 @@ clear_weakrefs(struct collection_state *state) { PyObject *op; WORKSTACK_FOR_EACH(&state->unreachable, op) { + if (PyGen_CheckExact(op) || + PyCoro_CheckExact(op) || + PyAsyncGen_CheckExact(op)) { + // Ensure any non-refcounted pointers to cyclic trash are converted + // to refcounted pointers. This prevents bugs where the generator is + // freed after its function object. + PyGenObject *gen = (PyGenObject *)op; + struct _PyInterpreterFrame *frame = &(gen->gi_iframe); + for (int i = 0; i < frame->stacktop; i++) { + gc_visit_stackref(frame->localsplus[i]); + } + } if (PyWeakref_Check(op)) { // Clear weakrefs that are themselves unreachable to ensure their // callbacks will not be executed later from a `tp_clear()` @@ -833,8 +878,8 @@ show_stats_each_generations(GCState *gcstate) } // Traversal callback for handle_resurrected_objects. -static int -visit_decref_unreachable(PyObject *op, void *data) +int +_Py_visit_decref_unreachable(PyObject *op, void *data) { if (gc_is_unreachable(op) && _PyObject_GC_IS_TRACKED(op)) { op->ob_ref_local -= 1; @@ -881,7 +926,7 @@ handle_resurrected_objects(struct collection_state *state) traverseproc traverse = Py_TYPE(op)->tp_traverse; (void) traverse(op, - (visitproc)visit_decref_unreachable, + (visitproc)_Py_visit_decref_unreachable, NULL); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 32b22aff14a768..b0e4021aa251f5 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1122,6 +1122,7 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); stack_pointer[-1 - oparg] = self; // Patch stack as it is used by _PY_FRAME_GENERAL method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); } @@ -2393,15 +2394,16 @@ _PyStackRef last_sent_val_st; _PyStackRef sub_iter_st; _PyStackRef none; - _PyStackRef value; + _PyStackRef *value; exc_value_st = stack_pointer[-1]; last_sent_val_st = stack_pointer[-2]; sub_iter_st = stack_pointer[-3]; + value = &stack_pointer[-2]; PyObject *exc_value = PyStackRef_AsPyObjectBorrow(exc_value_st); assert(throwflag); assert(exc_value && PyExceptionInstance_Check(exc_value)); if (PyErr_GivenExceptionMatches(exc_value, PyExc_StopIteration)) { - value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + *value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); PyStackRef_CLOSE(sub_iter_st); PyStackRef_CLOSE(last_sent_val_st); PyStackRef_CLOSE(exc_value_st); @@ -2413,7 +2415,6 @@ goto exception_unwind; } stack_pointer[-3] = none; - stack_pointer[-2] = value; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); @@ -4110,7 +4111,7 @@ INSTRUCTION_STATS(LOAD_ATTR_CLASS); static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 9, "incorrect cache size"); _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; /* Skip 1 cache entry */ // _CHECK_ATTR_CLASS @@ -4125,14 +4126,14 @@ /* Skip 2 cache entries */ // _LOAD_ATTR_CLASS { + attr = &stack_pointer[-1]; PyObject *descr = read_obj(&this_instr[6].cache); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - attr = PyStackRef_FromPyObjectNew(descr); + *attr = PyStackRef_FromPyObjectNew(descr); null = PyStackRef_NULL; PyStackRef_CLOSE(owner); } - stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -4502,7 +4503,7 @@ INSTRUCTION_STATS(LOAD_ATTR_SLOT); static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 9, "incorrect cache size"); _PyStackRef owner; - _PyStackRef attr; + _PyStackRef *attr; _PyStackRef null = PyStackRef_NULL; /* Skip 1 cache entry */ // _GUARD_TYPE_VERSION @@ -4515,6 +4516,7 @@ } // _LOAD_ATTR_SLOT { + attr = &stack_pointer[-1]; uint16_t index = read_u16(&this_instr[4].cache); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -4522,11 +4524,10 @@ DEOPT_IF(attr_o == NULL, LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - attr = PyStackRef_FromPyObjectNew(attr_o); + *attr = PyStackRef_FromPyObjectNew(attr_o); PyStackRef_CLOSE(owner); } /* Skip 5 cache entries */ - stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -6683,20 +6684,20 @@ INSTRUCTION_STATS(UNPACK_SEQUENCE_TWO_TUPLE); static_assert(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE == 1, "incorrect cache size"); _PyStackRef seq; - _PyStackRef val1; - _PyStackRef val0; + _PyStackRef *val1; + _PyStackRef *val0; /* Skip 1 cache entry */ seq = stack_pointer[-1]; + val1 = &stack_pointer[-1]; + val0 = &stack_pointer[0]; assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); DEOPT_IF(!PyTuple_CheckExact(seq_o), UNPACK_SEQUENCE); DEOPT_IF(PyTuple_GET_SIZE(seq_o) != 2, UNPACK_SEQUENCE); STAT_INC(UNPACK_SEQUENCE, hit); - val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); PyStackRef_CLOSE(seq); - stack_pointer[-1] = val1; - stack_pointer[0] = val0; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index a414b04fb6a5b1..5e9d19f986415b 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -705,12 +705,16 @@ } case _UNPACK_SEQUENCE_TWO_TUPLE: { - _Py_UopsSymbol *val1; - _Py_UopsSymbol *val0; - val1 = sym_new_not_null(ctx); - val0 = sym_new_not_null(ctx); - stack_pointer[-1] = val1; - stack_pointer[0] = val0; + _Py_UopsSymbol **val1; + _Py_UopsSymbol **val0; + val1 = &stack_pointer[-1]; + val0 = &stack_pointer[0]; + for (int _i = 1; --_i >= 0;) { + val1[_i] = sym_new_not_null(ctx); + } + for (int _i = 1; --_i >= 0;) { + val0[_i] = sym_new_not_null(ctx); + } stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; diff --git a/Python/pystate.c b/Python/pystate.c index 602b13e18c71ae..c4b63140e8ef57 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1582,13 +1582,6 @@ new_threadstate(PyInterpreterState *interp, int whence) PyMem_RawFree(new_tstate); } else { -#ifdef Py_GIL_DISABLED - if (_Py_atomic_load_int(&interp->gc.immortalize) == 0) { - // Immortalize objects marked as using deferred reference counting - // the first time a non-main thread is created. - _PyGC_ImmortalizeDeferredObjects(interp); - } -#endif } #ifdef Py_GIL_DISABLED From a90a074904fe43f3565e56ac94a69647327b75dd Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:38:00 +0800 Subject: [PATCH 03/25] add a steal --- Python/bytecodes.c | 7 +++---- Python/executor_cases.c.h | 3 +-- Python/generated_cases.c.h | 7 +++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 07b8c9b365717b..972d366c08e16b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -843,12 +843,11 @@ dummy_func( } inst(STORE_SUBSCR_DICT, (unused/1, value, dict_st, sub_st -- )) { - PyObject *sub = PyStackRef_AsPyObjectBorrow(sub_st); PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st); DEOPT_IF(!PyDict_CheckExact(dict)); STAT_INC(STORE_SUBSCR, hit); - int err = _PyDict_SetItem_Take2((PyDictObject *)dict, sub, PyStackRef_AsPyObjectSteal(value)); + int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub_st), PyStackRef_AsPyObjectSteal(value)); PyStackRef_CLOSE(dict_st); ERROR_IF(err, error); } @@ -1838,11 +1837,11 @@ dummy_func( } int err = 0; for (int i = 0; i < oparg; i++) { - PyObject *item = PyStackRef_AsPyObjectSteal(values[i]); + PyObject *item = PyStackRef_AsPyObjectBorrow(values[i]); if (err == 0) { err = PySet_Add(set_o, item); } - Py_DECREF(item); + PyStackRef_CLOSE(values[i]); } if (err != 0) { Py_DECREF(set_o); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index b14823b87eed96..b1ad8256f2eb6f 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1012,14 +1012,13 @@ sub_st = stack_pointer[-1]; dict_st = stack_pointer[-2]; value = stack_pointer[-3]; - PyObject *sub = PyStackRef_AsPyObjectBorrow(sub_st); PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st); if (!PyDict_CheckExact(dict)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); } STAT_INC(STORE_SUBSCR, hit); - int err = _PyDict_SetItem_Take2((PyDictObject *)dict, sub, PyStackRef_AsPyObjectSteal(value)); + int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub_st), PyStackRef_AsPyObjectSteal(value)); PyStackRef_CLOSE(dict_st); if (err) JUMP_TO_ERROR(); stack_pointer += -3; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index b0e4021aa251f5..a660ab2cae34f1 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -692,11 +692,11 @@ } int err = 0; for (int i = 0; i < oparg; i++) { - PyObject *item = PyStackRef_AsPyObjectSteal(values[i]); + PyObject *item = PyStackRef_AsPyObjectBorrow(values[i]); if (err == 0) { err = PySet_Add(set_o, item); } - Py_DECREF(item); + PyStackRef_CLOSE(values[i]); } if (err != 0) { Py_DECREF(set_o); @@ -6309,11 +6309,10 @@ sub_st = stack_pointer[-1]; dict_st = stack_pointer[-2]; value = stack_pointer[-3]; - PyObject *sub = PyStackRef_AsPyObjectBorrow(sub_st); PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st); DEOPT_IF(!PyDict_CheckExact(dict), STORE_SUBSCR); STAT_INC(STORE_SUBSCR, hit); - int err = _PyDict_SetItem_Take2((PyDictObject *)dict, sub, PyStackRef_AsPyObjectSteal(value)); + int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub_st), PyStackRef_AsPyObjectSteal(value)); PyStackRef_CLOSE(dict_st); if (err) goto pop_3_error; stack_pointer += -3; From e82c2dc7ac366c07d574c716d4cc538994132b0a Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sat, 29 Jun 2024 21:56:46 +0800 Subject: [PATCH 04/25] Conversion function should have steal variant --- Include/internal/pycore_stackref.h | 2 ++ Python/bytecodes.c | 26 +++++++++++++------------- Python/ceval.c | 25 +++++++++++++++++++++++-- Python/ceval_macros.h | 15 ++++++++++++--- Python/executor_cases.c.h | 22 +++++++++++----------- Python/generated_cases.c.h | 26 +++++++++++++------------- Tools/cases_generator/analyzer.py | 2 +- 7 files changed, 75 insertions(+), 43 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index e28a06fbca14f9..e24ed795f932be 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -156,6 +156,8 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj) // Converts a PyObject * to a PyStackRef, with a new reference +// IMPORTANT: The result of this operation must be immediately assigned to localsplus. +// There must be no interfering Py_DECREF calls or such between this operation and that. static inline _PyStackRef PyStackRef_FromPyObjectNew(PyObject *obj) { diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 972d366c08e16b..af90058a97dbea 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1766,7 +1766,7 @@ dummy_func( } inst(BUILD_STRING, (pieces[oparg] -- str)) { - STACKREFS_TO_PYOBJECTS(pieces, oparg, pieces_o); + STACKREFS_TO_PYOBJECTS_BORROW(pieces, oparg, pieces_o); if (CONVERSION_FAILED(pieces_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -1779,7 +1779,7 @@ dummy_func( } inst(BUILD_TUPLE, (values[oparg] -- tup)) { - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -1791,7 +1791,7 @@ dummy_func( } inst(BUILD_LIST, (values[oparg] -- list)) { - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -1851,7 +1851,7 @@ dummy_func( } inst(BUILD_MAP, (values[oparg*2] -- map)) { - STACKREFS_TO_PYOBJECTS(values, oparg*2, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg*2, values_o); if (CONVERSION_FAILED(values_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -1894,7 +1894,7 @@ dummy_func( assert(PyTuple_CheckExact(keys_o)); assert(PyTuple_GET_SIZE(keys_o) == (Py_ssize_t)oparg); - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -3383,7 +3383,7 @@ dummy_func( DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -3516,7 +3516,7 @@ dummy_func( total_args++; } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -3753,7 +3753,7 @@ dummy_func( PyTypeObject *tp = (PyTypeObject *)callable_o; DEOPT_IF(tp->tp_vectorcall == NULL); STAT_INC(CALL, hit); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -3823,7 +3823,7 @@ dummy_func( STAT_INC(CALL, hit); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable_o); /* res = func(self, args, nargs) */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -3867,7 +3867,7 @@ dummy_func( (PyCFunctionFastWithKeywords)(void(*)(void)) PyCFunction_GET_FUNCTION(callable_o); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -4031,7 +4031,7 @@ dummy_func( PyCFunctionFastWithKeywords cfunc = (PyCFunctionFastWithKeywords)(void(*)(void))meth->ml_meth; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -4112,7 +4112,7 @@ dummy_func( (PyCFunctionFast)(void(*)(void))meth->ml_meth; int nargs = total_args - 1; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); @@ -4196,7 +4196,7 @@ dummy_func( DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { DECREF_INPUTS(); ERROR_IF(true, error); diff --git a/Python/ceval.c b/Python/ceval.c index f4b3a417025c14..f1ebbbf372d584 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -698,7 +698,7 @@ extern void _PyUOpPrint(const _PyUOpInstruction *uop); PyObject ** -_PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch) +_PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch) { PyObject **result; if (nargs > MAX_STACKREF_SCRATCH) { @@ -718,6 +718,27 @@ _PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ssize_t nargs, PyObject return result; } +PyObject ** +_PyObjectArray_FromStackRefArraySteal(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch) +{ + PyObject **result; + if (nargs > MAX_STACKREF_SCRATCH) { + // +1 in case PY_VECTORCALL_ARGUMENTS_OFFSET is set. + result = PyMem_Malloc((nargs + 1) * sizeof(PyObject *)); + if (result == NULL) { + return NULL; + } + result++; + } + else { + result = scratch; + } + for (int i = 0; i < nargs; i++) { + result[i] = PyStackRef_AsPyObjectSteal(input[i]); + } + return result; +} + void _PyObjectArray_Free(PyObject **array, PyObject **scratch) { @@ -1520,7 +1541,7 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func, } else { assert(args != NULL); - STACKREFS_TO_PYOBJECTS((_PyStackRef *)args, argcount, args_o); + STACKREFS_TO_PYOBJECTS_STEAL((_PyStackRef *)args, argcount, args_o); if (args_o == NULL) { goto fail_pre_positional; } diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index f6d055a1dfaa9f..dbf5c0952e6d7d 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -456,12 +456,21 @@ do { \ #define MAX_STACKREF_SCRATCH 10 #ifdef Py_GIL_DISABLED -#define STACKREFS_TO_PYOBJECTS(ARGS, ARG_COUNT, NAME) \ +#define STACKREFS_TO_PYOBJECTS_BORROW(ARGS, ARG_COUNT, NAME) \ /* +1 because vectorcall might use -1 to write self */ \ PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \ - PyObject **NAME = _PyObjectArray_FromStackRefArray(ARGS, ARG_COUNT, NAME##_temp + 1); + PyObject **NAME = _PyObjectArray_FromStackRefArrayBorrow(ARGS, ARG_COUNT, NAME##_temp + 1); + +#define STACKREFS_TO_PYOBJECTS_STEAL(ARGS, ARG_COUNT, NAME) \ + /* +1 because vectorcall might use -1 to write self */ \ + PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \ + PyObject **NAME = _PyObjectArray_FromStackRefArraySteal(ARGS, ARG_COUNT, NAME##_temp + 1); #else -#define STACKREFS_TO_PYOBJECTS(ARGS, ARG_COUNT, NAME) \ +#define STACKREFS_TO_PYOBJECTS_BORROW(ARGS, ARG_COUNT, NAME) \ + PyObject **NAME = (PyObject **)ARGS; \ + assert(NAME != NULL); + +#define STACKREFS_TO_PYOBJECTS_STEAL(ARGS, ARG_COUNT, NAME) \ PyObject **NAME = (PyObject **)ARGS; \ assert(NAME != NULL); #endif diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index b1ad8256f2eb6f..5c6ab16403d48c 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1790,7 +1790,7 @@ _PyStackRef str; oparg = CURRENT_OPARG(); pieces = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(pieces, oparg, pieces_o); + STACKREFS_TO_PYOBJECTS_BORROW(pieces, oparg, pieces_o); if (CONVERSION_FAILED(pieces_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(pieces[_i]); @@ -1815,7 +1815,7 @@ _PyStackRef tup; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -1837,7 +1837,7 @@ _PyStackRef list; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -1904,7 +1904,7 @@ _PyStackRef map; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg*2]; - STACKREFS_TO_PYOBJECTS(values, oparg*2, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg*2, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg*2; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -1961,7 +1961,7 @@ PyObject *keys_o = PyStackRef_AsPyObjectBorrow(keys); assert(PyTuple_CheckExact(keys_o)); assert(PyTuple_GET_SIZE(keys_o) == (Py_ssize_t)oparg); - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -3577,7 +3577,7 @@ total_args++; } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -3989,7 +3989,7 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(CALL, hit); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -4090,7 +4090,7 @@ STAT_INC(CALL, hit); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable_o); /* res = func(self, args, nargs) */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -4147,7 +4147,7 @@ PyCFunctionFastWithKeywords cfunc = (PyCFunctionFastWithKeywords)(void(*)(void)) PyCFunction_GET_FUNCTION(callable_o); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -4356,7 +4356,7 @@ int nargs = total_args - 1; PyCFunctionFastWithKeywords cfunc = (PyCFunctionFastWithKeywords)(void(*)(void))meth->ml_meth; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -4473,7 +4473,7 @@ PyCFunctionFast cfunc = (PyCFunctionFast)(void(*)(void))meth->ml_meth; int nargs = total_args - 1; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index a660ab2cae34f1..1d386a92127ad3 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -601,7 +601,7 @@ PyObject *keys_o = PyStackRef_AsPyObjectBorrow(keys); assert(PyTuple_CheckExact(keys_o)); assert(PyTuple_GET_SIZE(keys_o) == (Py_ssize_t)oparg); - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -632,7 +632,7 @@ _PyStackRef *values; _PyStackRef list; values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -656,7 +656,7 @@ _PyStackRef *values; _PyStackRef map; values = &stack_pointer[-oparg*2]; - STACKREFS_TO_PYOBJECTS(values, oparg*2, values_o); + STACKREFS_TO_PYOBJECTS_BORROW(values, oparg*2, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg*2; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -742,7 +742,7 @@ _PyStackRef *pieces; _PyStackRef str; pieces = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(pieces, oparg, pieces_o); + STACKREFS_TO_PYOBJECTS_BORROW(pieces, oparg, pieces_o); if (CONVERSION_FAILED(pieces_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(pieces[_i]); @@ -769,7 +769,7 @@ _PyStackRef *values; _PyStackRef tup; values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); + STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); if (CONVERSION_FAILED(values_o)) { for (int _i = oparg; --_i >= 0;) { PyStackRef_CLOSE(values[_i]); @@ -867,7 +867,7 @@ DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1206,7 +1206,7 @@ PyTypeObject *tp = (PyTypeObject *)callable_o; DEOPT_IF(tp->tp_vectorcall == NULL, CALL); STAT_INC(CALL, hit); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1263,7 +1263,7 @@ STAT_INC(CALL, hit); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable_o); /* res = func(self, args, nargs) */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1326,7 +1326,7 @@ PyCFunctionFastWithKeywords cfunc = (PyCFunctionFastWithKeywords)(void(*)(void)) PyCFunction_GET_FUNCTION(callable_o); - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1640,7 +1640,7 @@ DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1798,7 +1798,7 @@ PyCFunctionFast cfunc = (PyCFunctionFast)(void(*)(void))meth->ml_meth; int nargs = total_args - 1; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -1861,7 +1861,7 @@ int nargs = total_args - 1; PyCFunctionFastWithKeywords cfunc = (PyCFunctionFastWithKeywords)(void(*)(void))meth->ml_meth; - STACKREFS_TO_PYOBJECTS(args, nargs, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); @@ -2035,7 +2035,7 @@ total_args++; } /* Callable is not a normal Python function */ - STACKREFS_TO_PYOBJECTS(args, total_args, args_o); + STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { PyStackRef_CLOSE(callable); PyStackRef_CLOSE(self_or_null); diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 6b1af1b59f14d8..a02bb7976e3900 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -426,7 +426,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "PyFloat_AS_DOUBLE", "_PyFrame_PushUnchecked", "Py_FatalError", - "STACKREFS_TO_PYOBJECTS", + "STACKREFS_TO_PYOBJECTS_BORROW", "STACKREFS_TO_PYOBJECTS_CLEANUP", "CONVERSION_FAILED", "_PyList_FromArraySteal", From 5df23bfe88acdac2bb60251f0782d0b9d5c409f3 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sat, 29 Jun 2024 22:18:56 +0800 Subject: [PATCH 05/25] remove a steal --- Python/bytecodes.c | 2 +- Python/generated_cases.c.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index af90058a97dbea..f193191806936e 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -3105,7 +3105,7 @@ dummy_func( inst(LOAD_SPECIAL, (owner -- attr, self_or_null)) { assert(oparg <= SPECIAL_MAX); - PyObject *owner_o = PyStackRef_AsPyObjectSteal(owner); + PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); PyObject *name = _Py_SpecialMethods[oparg].name; PyObject *self_or_null_o; attr = PyStackRef_FromPyObjectSteal(_PyObject_LookupSpecialMethod(owner_o, name, &self_or_null_o)); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 1d386a92127ad3..c4554b669ac3b6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5025,7 +5025,7 @@ _PyStackRef self_or_null; owner = stack_pointer[-1]; assert(oparg <= SPECIAL_MAX); - PyObject *owner_o = PyStackRef_AsPyObjectSteal(owner); + PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); PyObject *name = _Py_SpecialMethods[oparg].name; PyObject *self_or_null_o; attr = PyStackRef_FromPyObjectSteal(_PyObject_LookupSpecialMethod(owner_o, name, &self_or_null_o)); From 6b630662e1c5f6d3c3d8899e8bacf27aa1dd50ab Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Sun, 30 Jun 2024 21:46:41 +0800 Subject: [PATCH 06/25] Fix double decref in error path --- Lib/idlelib/idle_test/test_run.py | 32 +++---------------------------- Python/bytecodes.c | 4 +++- Python/executor_cases.c.h | 2 +- Python/generated_cases.c.h | 6 ++---- 4 files changed, 9 insertions(+), 35 deletions(-) diff --git a/Lib/idlelib/idle_test/test_run.py b/Lib/idlelib/idle_test/test_run.py index 83ecbffa2a197e..3cc30f83347299 100644 --- a/Lib/idlelib/idle_test/test_run.py +++ b/Lib/idlelib/idle_test/test_run.py @@ -396,38 +396,12 @@ def test_fatal_error(self): self.assertIn('IndexError', msg) eq(func.called, 2) - +a = [] +x = a.append class ExecRuncodeTest(unittest.TestCase): - @classmethod - def setUpClass(cls): - cls.addClassCleanup(setattr,run,'print_exception',run.print_exception) - cls.prt = Func() # Need reference. - run.print_exception = cls.prt - mockrpc = mock.Mock() - mockrpc.console.getvar = Func(result=False) - cls.ex = run.Executive(mockrpc) - - @classmethod - def tearDownClass(cls): - assert sys.excepthook == sys.__excepthook__ - def test_exceptions(self): - ex = self.ex - ex.runcode('1/0') - self.assertIs(ex.user_exc_info[0], ZeroDivisionError) - - self.addCleanup(setattr, sys, 'excepthook', sys.__excepthook__) - sys.excepthook = lambda t, e, tb: run.print_exception(t) - ex.runcode('1/0') - self.assertIs(self.prt.args[0], ZeroDivisionError) - - sys.excepthook = lambda: None - ex.runcode('1/0') - t, e, tb = ex.user_exc_info - self.assertIs(t, TypeError) - self.assertTrue(isinstance(e.__context__, ZeroDivisionError)) - + x(lambda a:a) if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f193191806936e..a93ebaa11f30f1 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -3385,7 +3385,9 @@ dummy_func( /* Callable is not a normal Python function */ STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { - DECREF_INPUTS(); + for (int i = 0; i < total_args; i++) { + PyStackRef_CLOSE(args[i]); + } ERROR_IF(true, error); } PyObject *res_o = PyObject_Vectorcall( diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 5c6ab16403d48c..0d3e67032a6a91 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -3211,7 +3211,7 @@ oparg = CURRENT_OPARG(); owner = stack_pointer[-1]; assert(oparg <= SPECIAL_MAX); - PyObject *owner_o = PyStackRef_AsPyObjectSteal(owner); + PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); PyObject *name = _Py_SpecialMethods[oparg].name; PyObject *self_or_null_o; attr = PyStackRef_FromPyObjectSteal(_PyObject_LookupSpecialMethod(owner_o, name, &self_or_null_o)); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index c4554b669ac3b6..508169e4759b74 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -869,10 +869,8 @@ /* Callable is not a normal Python function */ STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { - PyStackRef_CLOSE(callable); - PyStackRef_CLOSE(self_or_null); - for (int _i = oparg; --_i >= 0;) { - PyStackRef_CLOSE(args[_i]); + for (int i = 0; i < total_args; i++) { + PyStackRef_CLOSE(args[i]); } if (true) { stack_pointer += -2 - oparg; goto error; } } From 634adccada72489961fcb7feedfd5a766a812c27 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Mon, 1 Jul 2024 01:31:18 +0800 Subject: [PATCH 07/25] Revert "Fix double decref in error path" This reverts commit 6b630662e1c5f6d3c3d8899e8bacf27aa1dd50ab. --- Lib/idlelib/idle_test/test_run.py | 32 ++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/Lib/idlelib/idle_test/test_run.py b/Lib/idlelib/idle_test/test_run.py index 3cc30f83347299..83ecbffa2a197e 100644 --- a/Lib/idlelib/idle_test/test_run.py +++ b/Lib/idlelib/idle_test/test_run.py @@ -396,12 +396,38 @@ def test_fatal_error(self): self.assertIn('IndexError', msg) eq(func.called, 2) -a = [] -x = a.append + class ExecRuncodeTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.addClassCleanup(setattr,run,'print_exception',run.print_exception) + cls.prt = Func() # Need reference. + run.print_exception = cls.prt + mockrpc = mock.Mock() + mockrpc.console.getvar = Func(result=False) + cls.ex = run.Executive(mockrpc) + + @classmethod + def tearDownClass(cls): + assert sys.excepthook == sys.__excepthook__ + def test_exceptions(self): - x(lambda a:a) + ex = self.ex + ex.runcode('1/0') + self.assertIs(ex.user_exc_info[0], ZeroDivisionError) + + self.addCleanup(setattr, sys, 'excepthook', sys.__excepthook__) + sys.excepthook = lambda t, e, tb: run.print_exception(t) + ex.runcode('1/0') + self.assertIs(self.prt.args[0], ZeroDivisionError) + + sys.excepthook = lambda: None + ex.runcode('1/0') + t, e, tb = ex.user_exc_info + self.assertIs(t, TypeError) + self.assertTrue(isinstance(e.__context__, ZeroDivisionError)) + if __name__ == '__main__': unittest.main(verbosity=2) From 8cb139f4ad78cce6e34b63f441cbc18bb5e40da4 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 3 Jul 2024 17:55:41 +0800 Subject: [PATCH 08/25] Remove immortalize visitor --- Python/gc_free_threading.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index b1391267889ed7..e9b849739d51a8 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -1836,32 +1836,6 @@ custom_visitor_wrapper(const mi_heap_t *heap, const mi_heap_area_t *area, return true; } -// gh-117783: Immortalize objects that use deferred reference counting to -// temporarily work around scaling bottlenecks. -static bool -immortalize_visitor(const mi_heap_t *heap, const mi_heap_area_t *area, - void *block, size_t block_size, void *args) -{ - PyObject *op = op_from_block(block, args, false); - if (op != NULL && _PyObject_HasDeferredRefcount(op)) { - _Py_SetImmortal(op); - op->ob_gc_bits &= ~_PyGC_BITS_DEFERRED; - } - return true; -} - -void -_PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp) -{ - struct visitor_args args; - _PyEval_StopTheWorld(interp); - if (interp->gc.immortalize == 0) { - gc_visit_heaps(interp, &immortalize_visitor, &args); - interp->gc.immortalize = 1; - } - _PyEval_StartTheWorld(interp); -} - void PyUnstable_GC_VisitObjects(gcvisitobjects_t callback, void *arg) { From c260fc957b9b7957327941249953fd7d2cee2fc7 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 3 Jul 2024 18:15:35 +0800 Subject: [PATCH 09/25] fix the JIT builds --- Include/internal/pycore_ceval.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index b472d5d446b246..73c81fa20a5367 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -263,7 +263,8 @@ PyAPI_FUNC(PyObject *)_PyEval_MatchClass(PyThreadState *tstate, PyObject *subjec PyAPI_FUNC(PyObject *)_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, PyObject *keys); PyAPI_FUNC(int) _PyEval_UnpackIterableStackRef(PyThreadState *tstate, _PyStackRef v, int argcnt, int argcntafter, _PyStackRef *sp); PyAPI_FUNC(void) _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame); -PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch); +PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch); +PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArraySteal(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch); PyAPI_FUNC(void) _PyObjectArray_Free(PyObject **array, PyObject **scratch); From 700c2fdc0708df987a3d524831dfdd34c0ddeb0f Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 3 Jul 2024 20:49:52 +0800 Subject: [PATCH 10/25] fix buildbot bug --- Python/gc_free_threading.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index e9b849739d51a8..12d27b16aee445 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -310,7 +310,7 @@ gc_visit_stackref(_PyStackRef stackref) // being dead already. if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNull(stackref)) { PyObject *curr_o = PyStackRef_AsPyObjectBorrow(stackref); - if (!_Py_IsImmortal(curr_o)) { + if (_PyObject_GC_IS_TRACKED(curr_o)) { gc_add_refs(curr_o, 1); } } From cde931dd6cab4d17c7d61ad1d00a959b57b62ebd Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 3 Jul 2024 21:23:16 +0800 Subject: [PATCH 11/25] don't deref NULL --- Python/gc_free_threading.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 12d27b16aee445..5392e70caf8b2c 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -324,8 +324,11 @@ gc_visit_thread_stacks(PyInterpreterState *interp) _PyInterpreterFrame *curr_frame = p->current_frame; while (curr_frame != NULL) { PyCodeObject *co = (PyCodeObject *)curr_frame->f_executable; - for (int i = 0; i < co->co_nlocalsplus + co->co_stacksize; i++) { - gc_visit_stackref(curr_frame->localsplus[i]); + if (co != NULL && PyCode_Check(co)) { + for (int i = 0; + i < co->co_nlocalsplus + co->co_stacksize; i++) { + gc_visit_stackref(curr_frame->localsplus[i]); + } } curr_frame = curr_frame->previous; } @@ -585,14 +588,14 @@ deduce_unreachable_heap(PyInterpreterState *interp, // incoming references. gc_visit_heaps(interp, &update_refs, &state->base); - gc_visit_thread_stacks(interp); - #ifdef GC_DEBUG // Check that all objects are marked as unreachable and that the computed // reference count difference (stored in `ob_tid`) is non-negative. gc_visit_heaps(interp, &validate_gc_objects, &state->base); #endif + gc_visit_thread_stacks(interp); + // Transitively mark reachable objects by clearing the // _PyGC_BITS_UNREACHABLE flag. if (gc_visit_heaps(interp, &mark_heap_visitor, &state->base) < 0) { From 4e594207f7ff47089312cff36824d0a00be62a81 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 3 Jul 2024 23:10:59 +0800 Subject: [PATCH 12/25] Remove steal conversion, add list_fromstackrefsteal --- Include/internal/pycore_ceval.h | 1 - Include/internal/pycore_list.h | 2 ++ Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_metadata.h | 2 +- Objects/listobject.c | 23 +++++++++++++++++++++++ Python/bytecodes.c | 8 +------- Python/ceval.c | 21 --------------------- Python/ceval_macros.h | 9 --------- Python/executor_cases.c.h | 10 +--------- Python/generated_cases.c.h | 10 +--------- 10 files changed, 30 insertions(+), 58 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 73c81fa20a5367..0318c0d0d672a7 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -264,7 +264,6 @@ PyAPI_FUNC(PyObject *)_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, Py PyAPI_FUNC(int) _PyEval_UnpackIterableStackRef(PyThreadState *tstate, _PyStackRef v, int argcnt, int argcntafter, _PyStackRef *sp); PyAPI_FUNC(void) _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame); PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch); -PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArraySteal(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch); PyAPI_FUNC(void) _PyObjectArray_Free(PyObject **array, PyObject **scratch); diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h index 73695d10e0c372..84e5abdcb9ef85 100644 --- a/Include/internal/pycore_list.h +++ b/Include/internal/pycore_list.h @@ -59,6 +59,8 @@ typedef struct { } _PyListIterObject; PyAPI_FUNC(PyObject *)_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n); +PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const union _PyStackRef *src, Py_ssize_t n); + #ifdef __cplusplus } diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 7b495238d7a9f3..e14b32fb5b1182 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1005,7 +1005,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[264] = { [BINARY_SUBSCR_STR_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, [BINARY_SUBSCR_TUPLE_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, [BUILD_CONST_KEY_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, + [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SET] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SLICE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 2a2d6e923b7617..27279a652c6cf6 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -121,7 +121,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_COPY_FREE_VARS] = HAS_ARG_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, [_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG, - [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG, + [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_LIST_EXTEND] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_BUILD_MAP] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, diff --git a/Objects/listobject.c b/Objects/listobject.c index 9eae9626f7c1f1..8f7c2836841f71 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3203,6 +3203,29 @@ _PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n) return (PyObject *)list; } +PyObject * +_PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n) +{ + if (n == 0) { + return PyList_New(0); + } + + PyListObject *list = (PyListObject *)PyList_New(n); + if (list == NULL) { + for (Py_ssize_t i = 0; i < n; i++) { + PyStackRef_CLOSE(src[i]); + } + return NULL; + } + + PyObject **dst = list->ob_item; + for (Py_ssize_t i = 0; i < n; i++) { + dst[i] = PyStackRef_AsPyObjectSteal(src[i]); + } + + return (PyObject *)list; +} + /*[clinic input] list.index diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 0851c2a4e55c56..5a972f4f68af12 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1785,13 +1785,7 @@ dummy_func( } inst(BUILD_LIST, (values[oparg] -- list)) { - STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - DECREF_INPUTS(); - ERROR_IF(true, error); - } - PyObject *list_o = _PyList_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); ERROR_IF(list_o == NULL, error); list = PyStackRef_FromPyObjectSteal(list_o); } diff --git a/Python/ceval.c b/Python/ceval.c index a557663f16bf0f..4866903b2bf1bd 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -718,27 +718,6 @@ _PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyO return result; } -PyObject ** -_PyObjectArray_FromStackRefArraySteal(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch) -{ - PyObject **result; - if (nargs > MAX_STACKREF_SCRATCH) { - // +1 in case PY_VECTORCALL_ARGUMENTS_OFFSET is set. - result = PyMem_Malloc((nargs + 1) * sizeof(PyObject *)); - if (result == NULL) { - return NULL; - } - result++; - } - else { - result = scratch; - } - for (int i = 0; i < nargs; i++) { - result[i] = PyStackRef_AsPyObjectSteal(input[i]); - } - return result; -} - void _PyObjectArray_Free(PyObject **array, PyObject **scratch) { diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index 15067b9c612234..a0260b9d5349b0 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -459,19 +459,10 @@ do { \ /* +1 because vectorcall might use -1 to write self */ \ PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \ PyObject **NAME = _PyObjectArray_FromStackRefArrayBorrow(ARGS, ARG_COUNT, NAME##_temp + 1); - -#define STACKREFS_TO_PYOBJECTS_STEAL(ARGS, ARG_COUNT, NAME) \ - /* +1 because vectorcall might use -1 to write self */ \ - PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \ - PyObject **NAME = _PyObjectArray_FromStackRefArraySteal(ARGS, ARG_COUNT, NAME##_temp + 1); #else #define STACKREFS_TO_PYOBJECTS_BORROW(ARGS, ARG_COUNT, NAME) \ PyObject **NAME = (PyObject **)ARGS; \ assert(NAME != NULL); - -#define STACKREFS_TO_PYOBJECTS_STEAL(ARGS, ARG_COUNT, NAME) \ - PyObject **NAME = (PyObject **)ARGS; \ - assert(NAME != NULL); #endif #ifdef Py_GIL_DISABLED diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 188bd85980b26d..1274618e0ca14e 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1829,15 +1829,7 @@ _PyStackRef list; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - for (int _i = oparg; --_i >= 0;) { - PyStackRef_CLOSE(values[_i]); - } - if (true) JUMP_TO_ERROR(); - } - PyObject *list_o = _PyList_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); if (list_o == NULL) JUMP_TO_ERROR(); list = PyStackRef_FromPyObjectSteal(list_o); stack_pointer[-oparg] = list; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index a29891fae42a48..e95d9ac2fb6839 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -632,15 +632,7 @@ _PyStackRef *values; _PyStackRef list; values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS_STEAL(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - for (int _i = oparg; --_i >= 0;) { - PyStackRef_CLOSE(values[_i]); - } - if (true) { stack_pointer += -oparg; goto error; } - } - PyObject *list_o = _PyList_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); if (list_o == NULL) { stack_pointer += -oparg; goto error; } list = PyStackRef_FromPyObjectSteal(list_o); stack_pointer[-oparg] = list; From 98f9c0f6d8dff390d3bbad4e97f8225210814f10 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 01:18:07 +0800 Subject: [PATCH 13/25] Silence warnings --- Include/internal/pycore_list.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h index 84e5abdcb9ef85..30c9ffd4fe3f1b 100644 --- a/Include/internal/pycore_list.h +++ b/Include/internal/pycore_list.h @@ -9,6 +9,7 @@ extern "C" { #endif #include "pycore_freelist.h" // _PyFreeListState +#include "pycore_stackref.h" // _PyStackRef PyAPI_FUNC(PyObject*) _PyList_Extend(PyListObject *, PyObject *); extern void _PyList_DebugMallocStats(FILE *out); @@ -59,7 +60,7 @@ typedef struct { } _PyListIterObject; PyAPI_FUNC(PyObject *)_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n); -PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const union _PyStackRef *src, Py_ssize_t n); +PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n); #ifdef __cplusplus From 32aaf2b2d77bee6c80f408d935935b7928c7d832 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 01:34:21 +0800 Subject: [PATCH 14/25] Fix error case in CALL --- Python/bytecodes.c | 1 + Python/generated_cases.c.h | 1 + 2 files changed, 2 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 5a972f4f68af12..080e0645b5ea89 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -3373,6 +3373,7 @@ dummy_func( /* Callable is not a normal Python function */ STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { + PyStackRef_CLOSE(callable); for (int i = 0; i < total_args; i++) { PyStackRef_CLOSE(args[i]); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index e95d9ac2fb6839..b9cc4f48fd0b17 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -853,6 +853,7 @@ /* Callable is not a normal Python function */ STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o); if (CONVERSION_FAILED(args_o)) { + PyStackRef_CLOSE(callable); for (int i = 0; i < total_args; i++) { PyStackRef_CLOSE(args[i]); } From 41c621827a42dbd63991ffa312ba8663b1414571 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 01:50:58 +0800 Subject: [PATCH 15/25] remove all temporary immortalization --- Include/internal/pycore_gc.h | 8 ----- Modules/_testinternalcapi.c | 14 -------- Objects/codeobject.c | 65 ------------------------------------ Python/bltinmodule.c | 13 -------- Python/gc_free_threading.c | 4 --- 5 files changed, 104 deletions(-) diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index d2460a8f083562..498c60418762cb 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -344,14 +344,6 @@ struct _gc_runtime_state { collections, and are awaiting to undergo a full collection for the first time. */ Py_ssize_t long_lived_pending; - - /* gh-117783: Deferred reference counting is not fully implemented yet, so - as a temporary measure we treat objects using deferred reference - counting as immortal. The value may be zero, one, or a negative number: - 0: immortalize deferred RC objects once the first thread is created - 1: immortalize all deferred RC objects immediately - <0: suppressed; don't immortalize objects */ - int immortalize; #endif }; diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 6e6386bc044dc3..4a72105cbad100 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1968,27 +1968,13 @@ get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * suppress_immortalization(PyObject *self, PyObject *value) { -#ifdef Py_GIL_DISABLED - int suppress = PyObject_IsTrue(value); - if (suppress < 0) { - return NULL; - } - PyInterpreterState *interp = PyInterpreterState_Get(); - // Subtract two to suppress immortalization (so that 1 -> -1) - _Py_atomic_add_int(&interp->gc.immortalize, suppress ? -2 : 2); -#endif Py_RETURN_NONE; } static PyObject * get_immortalize_deferred(PyObject *self, PyObject *Py_UNUSED(ignored)) { -#ifdef Py_GIL_DISABLED - PyInterpreterState *interp = PyInterpreterState_Get(); - return PyBool_FromLong(_Py_atomic_load_int(&interp->gc.immortalize) >= 0); -#else Py_RETURN_FALSE; -#endif } static PyObject * diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 7493280c898750..d027c505929903 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -105,16 +105,6 @@ PyCode_ClearWatcher(int watcher_id) static int should_intern_string(PyObject *o) { -#ifdef Py_GIL_DISABLED - // The free-threaded build interns (and immortalizes) all string constants - // unless we've disabled immortalizing objects that use deferred reference - // counting. - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (_Py_atomic_load_int(&interp->gc.immortalize) < 0) { - return 1; - } -#endif - // compute if s matches [a-zA-Z0-9_] const unsigned char *s, *e; @@ -130,10 +120,6 @@ should_intern_string(PyObject *o) return 1; } -#ifdef Py_GIL_DISABLED -static PyObject *intern_one_constant(PyObject *op); -#endif - static int intern_strings(PyObject *tuple) { @@ -235,27 +221,6 @@ intern_constants(PyObject *tuple, int *modified) } Py_DECREF(tmp); } - - // Intern non-string constants in the free-threaded build, but only if - // we are also immortalizing objects that use deferred reference - // counting. - PyThreadState *tstate = PyThreadState_GET(); - if (!_Py_IsImmortal(v) && !PyCode_Check(v) && - !PyUnicode_CheckExact(v) && - _Py_atomic_load_int(&tstate->interp->gc.immortalize) >= 0) - { - PyObject *interned = intern_one_constant(v); - if (interned == NULL) { - return -1; - } - else if (interned != v) { - PyTuple_SET_ITEM(tuple, i, interned); - Py_SETREF(v, interned); - if (modified) { - *modified = 1; - } - } - } #endif } return 0; @@ -2495,36 +2460,6 @@ _PyCode_ConstantKey(PyObject *op) } #ifdef Py_GIL_DISABLED -static PyObject * -intern_one_constant(PyObject *op) -{ - PyInterpreterState *interp = _PyInterpreterState_GET(); - _Py_hashtable_t *consts = interp->code_state.constants; - - assert(!PyUnicode_CheckExact(op)); // strings are interned separately - - _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(consts, op); - if (entry == NULL) { - if (_Py_hashtable_set(consts, op, op) != 0) { - return NULL; - } - -#ifdef Py_REF_DEBUG - Py_ssize_t refcnt = Py_REFCNT(op); - if (refcnt != 1) { - // Adjust the reftotal to account for the fact that we only - // restore a single reference in _PyCode_Fini. - _Py_AddRefTotal(_PyThreadState_GET(), -(refcnt - 1)); - } -#endif - - _Py_SetImmortal(op); - return op; - } - - assert(_Py_IsImmortal(entry->value)); - return (PyObject *)entry->value; -} static int compare_constants(const void *key1, const void *key2) { diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 6e50623cafa4ed..d58cb928a4996e 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -866,21 +866,8 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, if (str == NULL) goto error; -#ifdef Py_GIL_DISABLED - // gh-118527: Disable immortalization of code constants for explicit - // compile() calls to get consistent frozen outputs between the default - // and free-threaded builds. - // Subtract two to suppress immortalization (so that 1 -> -1) - PyInterpreterState *interp = _PyInterpreterState_GET(); - _Py_atomic_add_int(&interp->gc.immortalize, -2); -#endif - result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize); -#ifdef Py_GIL_DISABLED - _Py_atomic_add_int(&interp->gc.immortalize, 2); -#endif - Py_XDECREF(source_copy); goto finally; diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 5392e70caf8b2c..2a26323b566849 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -755,10 +755,6 @@ _PyGC_Init(PyInterpreterState *interp) { GCState *gcstate = &interp->gc; - // gh-117783: immortalize objects that would use deferred refcounting - // once the first non-main thread is created (but not in subinterpreters). - gcstate->immortalize = _Py_IsMainInterpreter(interp) ? 0 : -1; - gcstate->garbage = PyList_New(0); if (gcstate->garbage == NULL) { return _PyStatus_NO_MEMORY(); From dca2ec39c544a090e43916da2cbbc78d21bc29d7 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 02:03:51 +0800 Subject: [PATCH 16/25] Revert "remove all temporary immortalization" This reverts commit 41c621827a42dbd63991ffa312ba8663b1414571. --- Include/internal/pycore_gc.h | 8 +++++ Modules/_testinternalcapi.c | 14 ++++++++ Objects/codeobject.c | 65 ++++++++++++++++++++++++++++++++++++ Python/bltinmodule.c | 13 ++++++++ Python/gc_free_threading.c | 4 +++ 5 files changed, 104 insertions(+) diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index 498c60418762cb..d2460a8f083562 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -344,6 +344,14 @@ struct _gc_runtime_state { collections, and are awaiting to undergo a full collection for the first time. */ Py_ssize_t long_lived_pending; + + /* gh-117783: Deferred reference counting is not fully implemented yet, so + as a temporary measure we treat objects using deferred reference + counting as immortal. The value may be zero, one, or a negative number: + 0: immortalize deferred RC objects once the first thread is created + 1: immortalize all deferred RC objects immediately + <0: suppressed; don't immortalize objects */ + int immortalize; #endif }; diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 4a72105cbad100..6e6386bc044dc3 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1968,13 +1968,27 @@ get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * suppress_immortalization(PyObject *self, PyObject *value) { +#ifdef Py_GIL_DISABLED + int suppress = PyObject_IsTrue(value); + if (suppress < 0) { + return NULL; + } + PyInterpreterState *interp = PyInterpreterState_Get(); + // Subtract two to suppress immortalization (so that 1 -> -1) + _Py_atomic_add_int(&interp->gc.immortalize, suppress ? -2 : 2); +#endif Py_RETURN_NONE; } static PyObject * get_immortalize_deferred(PyObject *self, PyObject *Py_UNUSED(ignored)) { +#ifdef Py_GIL_DISABLED + PyInterpreterState *interp = PyInterpreterState_Get(); + return PyBool_FromLong(_Py_atomic_load_int(&interp->gc.immortalize) >= 0); +#else Py_RETURN_FALSE; +#endif } static PyObject * diff --git a/Objects/codeobject.c b/Objects/codeobject.c index d027c505929903..7493280c898750 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -105,6 +105,16 @@ PyCode_ClearWatcher(int watcher_id) static int should_intern_string(PyObject *o) { +#ifdef Py_GIL_DISABLED + // The free-threaded build interns (and immortalizes) all string constants + // unless we've disabled immortalizing objects that use deferred reference + // counting. + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_Py_atomic_load_int(&interp->gc.immortalize) < 0) { + return 1; + } +#endif + // compute if s matches [a-zA-Z0-9_] const unsigned char *s, *e; @@ -120,6 +130,10 @@ should_intern_string(PyObject *o) return 1; } +#ifdef Py_GIL_DISABLED +static PyObject *intern_one_constant(PyObject *op); +#endif + static int intern_strings(PyObject *tuple) { @@ -221,6 +235,27 @@ intern_constants(PyObject *tuple, int *modified) } Py_DECREF(tmp); } + + // Intern non-string constants in the free-threaded build, but only if + // we are also immortalizing objects that use deferred reference + // counting. + PyThreadState *tstate = PyThreadState_GET(); + if (!_Py_IsImmortal(v) && !PyCode_Check(v) && + !PyUnicode_CheckExact(v) && + _Py_atomic_load_int(&tstate->interp->gc.immortalize) >= 0) + { + PyObject *interned = intern_one_constant(v); + if (interned == NULL) { + return -1; + } + else if (interned != v) { + PyTuple_SET_ITEM(tuple, i, interned); + Py_SETREF(v, interned); + if (modified) { + *modified = 1; + } + } + } #endif } return 0; @@ -2460,6 +2495,36 @@ _PyCode_ConstantKey(PyObject *op) } #ifdef Py_GIL_DISABLED +static PyObject * +intern_one_constant(PyObject *op) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + _Py_hashtable_t *consts = interp->code_state.constants; + + assert(!PyUnicode_CheckExact(op)); // strings are interned separately + + _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(consts, op); + if (entry == NULL) { + if (_Py_hashtable_set(consts, op, op) != 0) { + return NULL; + } + +#ifdef Py_REF_DEBUG + Py_ssize_t refcnt = Py_REFCNT(op); + if (refcnt != 1) { + // Adjust the reftotal to account for the fact that we only + // restore a single reference in _PyCode_Fini. + _Py_AddRefTotal(_PyThreadState_GET(), -(refcnt - 1)); + } +#endif + + _Py_SetImmortal(op); + return op; + } + + assert(_Py_IsImmortal(entry->value)); + return (PyObject *)entry->value; +} static int compare_constants(const void *key1, const void *key2) { diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index d58cb928a4996e..6e50623cafa4ed 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -866,8 +866,21 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, if (str == NULL) goto error; +#ifdef Py_GIL_DISABLED + // gh-118527: Disable immortalization of code constants for explicit + // compile() calls to get consistent frozen outputs between the default + // and free-threaded builds. + // Subtract two to suppress immortalization (so that 1 -> -1) + PyInterpreterState *interp = _PyInterpreterState_GET(); + _Py_atomic_add_int(&interp->gc.immortalize, -2); +#endif + result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize); +#ifdef Py_GIL_DISABLED + _Py_atomic_add_int(&interp->gc.immortalize, 2); +#endif + Py_XDECREF(source_copy); goto finally; diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 2a26323b566849..5392e70caf8b2c 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -755,6 +755,10 @@ _PyGC_Init(PyInterpreterState *interp) { GCState *gcstate = &interp->gc; + // gh-117783: immortalize objects that would use deferred refcounting + // once the first non-main thread is created (but not in subinterpreters). + gcstate->immortalize = _Py_IsMainInterpreter(interp) ? 0 : -1; + gcstate->garbage = PyList_New(0); if (gcstate->garbage == NULL) { return _PyStatus_NO_MEMORY(); From 39e8a56db0ec34d5f8069a1f233acd0939e72583 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 15:54:13 +0800 Subject: [PATCH 17/25] Apply suggestions --- Include/internal/pycore_gc.h | 4 +-- Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_metadata.h | 2 +- Python/frame.c | 15 +----------- Python/gc_free_threading.c | 30 +++++++++++++++++++---- Tools/cases_generator/analyzer.py | 1 + 6 files changed, 31 insertions(+), 23 deletions(-) diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index d2460a8f083562..615dc60fa27ac1 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -388,8 +388,8 @@ extern void _Py_RunGC(PyThreadState *tstate); extern void _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp); #endif -extern int _Py_visit_decref(PyObject *op, void *arg); -extern int _Py_visit_decref_unreachable(PyObject *op, void *data); +// Functions to clear generator frames +extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg); #ifdef __cplusplus } diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index e14b32fb5b1182..7b495238d7a9f3 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1005,7 +1005,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[264] = { [BINARY_SUBSCR_STR_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, [BINARY_SUBSCR_TUPLE_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, [BUILD_CONST_KEY_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, + [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, [BUILD_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SET] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SLICE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 27279a652c6cf6..2a2d6e923b7617 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -121,7 +121,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_COPY_FREE_VARS] = HAS_ARG_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, [_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG, - [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, + [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG, [_LIST_EXTEND] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_BUILD_MAP] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, diff --git a/Python/frame.c b/Python/frame.c index 19f99b50d94b83..1fbe2442d87be4 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -15,20 +15,7 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg) Py_VISIT(frame->f_locals); Py_VISIT(frame->f_funcobj); Py_VISIT(_PyFrame_GetCode(frame)); - /* locals */ - _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); - int i = 0; - /* locals and stack */ - for (; i stacktop; i++) { -#ifdef Py_GIL_DISABLED - if (PyStackRef_IsDeferred(locals[i]) && - (visit == _Py_visit_decref || visit == _Py_visit_decref_unreachable)) { - continue; - } -#endif - Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); - } - return 0; + return _PyGC_VisitFrameStack(frame, visit, arg); } PyFrameObject * diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 5392e70caf8b2c..64fb17d2ec6c3a 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -389,8 +389,8 @@ process_delayed_frees(PyInterpreterState *interp) } // Subtract an incoming reference from the computed "gc_refs" refcount. -int -_Py_visit_decref(PyObject *op, void *arg) +static int +visit_decref(PyObject *op, void *arg) { if (_PyObject_GC_IS_TRACKED(op) && !_Py_IsImmortal(op)) { // If update_refs hasn't reached this object yet, mark it @@ -457,7 +457,7 @@ update_refs(const mi_heap_t *heap, const mi_heap_area_t *area, // Subtract internal references from ob_tid. Objects with ob_tid > 0 // are directly reachable from outside containers, and so can't be // collected. - Py_TYPE(op)->tp_traverse(op, _Py_visit_decref, NULL); + Py_TYPE(op)->tp_traverse(op, visit_decref, NULL); return true; } @@ -882,7 +882,7 @@ show_stats_each_generations(GCState *gcstate) // Traversal callback for handle_resurrected_objects. int -_Py_visit_decref_unreachable(PyObject *op, void *data) +visit_decref_unreachable(PyObject *op, void *data) { if (gc_is_unreachable(op) && _PyObject_GC_IS_TRACKED(op)) { op->ob_ref_local -= 1; @@ -890,6 +890,26 @@ _Py_visit_decref_unreachable(PyObject *op, void *data) return 0; } + +int +_PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) +{ + /* locals */ + _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); + int i = 0; + /* locals and stack */ + for (; i stacktop; i++) { +#ifdef Py_GIL_DISABLED + if (PyStackRef_IsDeferred(locals[i]) && + (visit == visit_decref || visit == visit_decref_unreachable)) { + continue; + } +#endif + Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); + } + return 0; +} + // Handle objects that may have resurrected after a call to 'finalize_garbage'. static int handle_resurrected_objects(struct collection_state *state) @@ -929,7 +949,7 @@ handle_resurrected_objects(struct collection_state *state) traverseproc traverse = Py_TYPE(op)->tp_traverse; (void) traverse(op, - (visitproc)_Py_visit_decref_unreachable, + (visitproc)visit_decref_unreachable, NULL); } diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 4b256edb64f2cb..36388a85331820 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -432,6 +432,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "_PyList_FromArraySteal", "_PyTuple_FromArraySteal", "_PyTuple_FromStackRefSteal", + "_PyList_FromStackRefSteal", ) ESCAPING_FUNCTIONS = ( From 3e14b1ca60e29d3444553f36758f3f8428ea5088 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 4 Jul 2024 16:07:53 +0800 Subject: [PATCH 18/25] fix non-free-threaded --- Python/gc.c | 13 +++++++++++++ Python/gc_free_threading.c | 3 --- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Python/gc.c b/Python/gc.c index 38a0da91a97510..60e9e779125fc1 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -534,6 +534,19 @@ visit_decref(PyObject *op, void *parent) return 0; } +int +_PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) +{ + /* locals */ + _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); + int i = 0; + /* locals and stack */ + for (; i stacktop; i++) { + Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); + } + return 0; +} + /* Subtract internal references from gc_refs. After this, gc_refs is >= 0 * for all objects in containers, and is GC_REACHABLE for all tracked gc * objects not in containers. The ones with gc_refs > 0 are directly diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 64fb17d2ec6c3a..af6861fdcc35ab 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -890,7 +890,6 @@ visit_decref_unreachable(PyObject *op, void *data) return 0; } - int _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) { @@ -899,12 +898,10 @@ _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) int i = 0; /* locals and stack */ for (; i stacktop; i++) { -#ifdef Py_GIL_DISABLED if (PyStackRef_IsDeferred(locals[i]) && (visit == visit_decref || visit == visit_decref_unreachable)) { continue; } -#endif Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); } return 0; From 07f78cea8e1af20924441e7efc17a3e6b1c464c0 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Tue, 9 Jul 2024 20:38:44 +0800 Subject: [PATCH 19/25] fix bytecodes.c to use arrays instead of pointers --- Python/bytecodes.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 183067469f8ba5..28de98589ecd90 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1276,7 +1276,7 @@ dummy_func( } } - tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value: _PyStackRef *)) { + tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value[1])) { PyObject *exc_value = PyStackRef_AsPyObjectBorrow(exc_value_st); assert(throwflag); assert(exc_value && PyExceptionInstance_Check(exc_value)); @@ -1384,7 +1384,7 @@ dummy_func( macro(UNPACK_SEQUENCE) = _SPECIALIZE_UNPACK_SEQUENCE + _UNPACK_SEQUENCE; - inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1: _PyStackRef *, val0: _PyStackRef *)) { + inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1[1], val0[1])) { assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); DEOPT_IF(!PyTuple_CheckExact(seq_o)); @@ -2216,7 +2216,7 @@ dummy_func( _LOAD_ATTR_WITH_HINT + unused/5; - split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr: _PyStackRef *, null if (oparg & 1))) { + split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr[1], null if (oparg & 1))) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2243,7 +2243,7 @@ dummy_func( } - split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr: _PyStackRef *, null if (oparg & 1))) { + split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr[1], null if (oparg & 1))) { STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); *attr = PyStackRef_FromPyObjectNew(descr); From f4b40223bbaa764da44e9e326bf5d9e4475d997f Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 18 Jul 2024 22:14:38 +0800 Subject: [PATCH 20/25] Automatically flush to stack new --- Include/internal/pycore_frame.h | 2 +- Python/executor_cases.c.h | 42 ++++++++++++++++++++++ Python/gc.c | 8 ++--- Python/gc_free_threading.c | 14 ++++---- Python/generated_cases.c.h | 41 ++++++++++++++++++++- Tools/cases_generator/analyzer.py | 31 ++++++++++++++-- Tools/cases_generator/generators_common.py | 14 +++++++- Tools/cases_generator/stack.py | 25 +++++++++++++ 8 files changed, 158 insertions(+), 19 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index b29ad4053140c3..d3d745574667aa 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -131,7 +131,7 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame * #ifdef Py_GIL_DISABLED PyCodeObject *co = (PyCodeObject *)dest->f_executable; - for (int i = src->stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) { + for (int i = stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) { dest->localsplus[i] = PyStackRef_NULL; } #endif diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 7daf9d07091208..c7f0c99ae71758 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -173,6 +173,9 @@ _PyStackRef value; oparg = CURRENT_OPARG(); value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = value; + #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3066,6 +3069,9 @@ assert(seq); assert(it->it_index < PyList_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = next; + #endif /* Py_GIL_DISABLED */ stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3113,6 +3119,9 @@ assert(seq); assert(it->it_index < PyTuple_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = next; + #endif /* Py_GIL_DISABLED */ stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3320,6 +3329,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3341,6 +3353,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3360,6 +3375,9 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ stack_pointer[-1] = attr; break; } @@ -3376,6 +3394,9 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ stack_pointer[-1] = attr; break; } @@ -3406,6 +3427,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3518,7 +3542,13 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1 - oparg] = self; + #endif /* Py_GIL_DISABLED */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-2 - oparg] = method; + #endif /* Py_GIL_DISABLED */ stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); @@ -3616,7 +3646,13 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1 - oparg] = self; + #endif /* Py_GIL_DISABLED */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-2 - oparg] = func; + #endif /* Py_GIL_DISABLED */ PyStackRef_CLOSE(callable); stack_pointer[-2 - oparg] = func; stack_pointer[-1 - oparg] = self; @@ -4866,6 +4902,9 @@ _PyStackRef value; PyObject *ptr = (PyObject *)CURRENT_OPERAND(); value = PyStackRef_FromPyObjectNew(ptr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = value; + #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4898,6 +4937,9 @@ _PyStackRef null; PyObject *ptr = (PyObject *)CURRENT_OPERAND(); value = PyStackRef_FromPyObjectNew(ptr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = value; + #endif /* Py_GIL_DISABLED */ null = PyStackRef_NULL; stack_pointer[0] = value; stack_pointer[1] = null; diff --git a/Python/gc.c b/Python/gc.c index 60e9e779125fc1..291c1079f5f9c3 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -537,12 +537,10 @@ visit_decref(PyObject *op, void *parent) int _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) { - /* locals */ - _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); - int i = 0; + _PyStackRef *i = frame->localsplus; /* locals and stack */ - for (; i stacktop; i++) { - Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); + for (; i < frame->stackpointer; i++) { + Py_VISIT(PyStackRef_AsPyObjectBorrow(*i)); } return 0; } diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 437771fbe42af0..e86dda29dddd24 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -703,8 +703,8 @@ clear_weakrefs(struct collection_state *state) // freed after its function object. PyGenObject *gen = (PyGenObject *)op; struct _PyInterpreterFrame *frame = &(gen->gi_iframe); - for (int i = 0; i < frame->stacktop; i++) { - gc_visit_stackref(frame->localsplus[i]); + for (_PyStackRef *i = frame->localsplus; i < frame->stackpointer; i++) { + gc_visit_stackref(*i); } } if (PyWeakref_Check(op)) { @@ -939,16 +939,14 @@ visit_decref_unreachable(PyObject *op, void *data) int _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) { - /* locals */ - _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); - int i = 0; + _PyStackRef *i = frame->localsplus; /* locals and stack */ - for (; i stacktop; i++) { - if (PyStackRef_IsDeferred(locals[i]) && + for (; i < frame->stackpointer; i++) { + if (PyStackRef_IsDeferred(*i) && (visit == visit_decref || visit == visit_decref_unreachable)) { continue; } - Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i])); + Py_VISIT(PyStackRef_AsPyObjectBorrow(*i)); } return 0; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0fb8c371a14b50..33003e80909a09 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -992,7 +992,13 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1 - oparg] = self; + #endif /* Py_GIL_DISABLED */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-2 - oparg] = func; + #endif /* Py_GIL_DISABLED */ PyStackRef_CLOSE(callable); } // flush @@ -1101,7 +1107,13 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1 - oparg] = self; + #endif /* Py_GIL_DISABLED */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-2 - oparg] = method; + #endif /* Py_GIL_DISABLED */ stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); @@ -3177,6 +3189,9 @@ assert(seq); assert(it->it_index < PyList_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = next; + #endif /* Py_GIL_DISABLED */ } stack_pointer[0] = next; stack_pointer += 1; @@ -3270,6 +3285,9 @@ assert(seq); assert(it->it_index < PyTuple_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = next; + #endif /* Py_GIL_DISABLED */ } stack_pointer[0] = next; stack_pointer += 1; @@ -4113,7 +4131,7 @@ STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); *attr = PyStackRef_FromPyObjectNew(descr); - null = PyStackRef_NULL; + if (oparg & 1) null = PyStackRef_NULL; PyStackRef_CLOSE(owner); } if (oparg & 1) stack_pointer[0] = null; @@ -4236,6 +4254,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; } stack_pointer[-1] = attr; @@ -4272,6 +4293,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; } stack_pointer[-1] = attr; @@ -4320,6 +4344,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ self = owner; } stack_pointer[-1] = attr; @@ -4398,6 +4425,9 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ } stack_pointer[-1] = attr; DISPATCH(); @@ -4440,6 +4470,9 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* Py_GIL_DISABLED */ } stack_pointer[-1] = attr; DISPATCH(); @@ -4621,6 +4654,9 @@ INSTRUCTION_STATS(LOAD_CONST); _PyStackRef value; value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = value; + #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5636,6 +5672,9 @@ // _LOAD_CONST { value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = value; + #endif /* Py_GIL_DISABLED */ } // _RETURN_VALUE retval = value; diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 7776b961789f26..9b145b3fb9b7eb 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -156,6 +156,7 @@ class Uop: stack: StackEffect caches: list[CacheEntry] body: list[lexer.Token] + token_requires_flush: list[bool] # bitmap to body properties: Properties _size: int = -1 implicitly_created: bool = False @@ -345,6 +346,24 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]: return [CacheEntry(i.name, int(i.size)) for i in caches] +def analyze_specials_requiring_flush(name: str, op: list[lexer.Token], outputs: list[StackItem]) -> list[bool]: + res = [] + for idx, token in enumerate(op): + if token.kind == "IDENTIFIER" and token.text == "PyStackRef_FromPyObjectNew": + should_flush = True + assert op[idx-1].kind == "EQUALS", (f"{name} Result of a specials that is flushed" + " must be written to a stack variable directly") + # Scan to look for the first thing it's assigned to + for assgn_target in reversed(op[:idx-1]): + out_names = [out.name for out in outputs] + if assgn_target.kind == "IDENTIFIER": + should_flush = assgn_target.text in out_names + break + res.append(should_flush) + else: + res.append(False) + return res + def variable_used(node: parser.InstDef, name: str) -> bool: """Determine whether a variable with a given name is used in a node.""" return any( @@ -619,13 +638,15 @@ def compute_properties(op: parser.InstDef) -> Properties: def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uops: dict[str, Uop]) -> Uop: + stack = analyze_stack(op) result = Uop( name=name, context=op.context, annotations=op.annotations, - stack=analyze_stack(op), + stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, + token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=compute_properties(op), ) if effect_depends_on_oparg_1(op) and "split" in op.annotations: @@ -636,13 +657,15 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo if properties.oparg: # May not need oparg anymore properties.oparg = any(token.text == "oparg" for token in op.block.tokens) + stack = analyze_stack(op, bit) rep = Uop( name=name_x, context=op.context, annotations=op.annotations, - stack=analyze_stack(op, bit), + stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, + token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=properties, ) rep.replicates = result @@ -658,13 +681,15 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo properties = compute_properties(op) properties.oparg = False properties.const_oparg = oparg + stack = analyze_stack(op) rep = Uop( name=name_x, context=op.context, annotations=op.annotations, - stack=analyze_stack(op), + stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, + token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=properties, ) rep.replicates = result diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 9314bb9e79687f..165a82dcc0ccfd 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -207,11 +207,23 @@ def emit_tokens( return tkn_iter = iter(tkns) out.start_line() - for tkn in tkn_iter: + var_to_flush = None + for idx, tkn in enumerate(tkn_iter): if tkn.kind == "IDENTIFIER" and tkn.text in replacement_functions: replacement_functions[tkn.text](out, tkn, tkn_iter, uop, stack, inst) else: out.emit(tkn) + if uop.token_requires_flush[idx]: + assert tkns[idx-3].kind == "IDENTIFIER" + var_to_flush = tkns[idx-3].text + if var_to_flush and tkn.kind == "SEMI": + txt = stack.write_variable_to_stack(out, var_to_flush) + if txt: + out.start_line() + out.emit("#ifdef Py_GIL_DISABLED /* flush specials */ \n") + out.emit(txt) + out.emit("#endif /* Py_GIL_DISABLED */\n") + var_to_flush = None def cflags(p: Properties) -> str: diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index f497fa34dfced4..1e1aca6ce14699 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -213,6 +213,31 @@ def flush(self, out: CWriter, cast_type: str = "uintptr_t", extract_bits: bool = self.peek_offset.clear() out.start_line() + def write_variable_to_stack(self, out: CWriter, var: str, cast_type: str = "uintptr_t", extract_bits: bool = False) -> str: + out.start_line() + var = [variable for variable in self.variables if variable.name == var] + assert len(var) == 1 + to_write_var = var[0] + txt = "" + for var in self.variables: + if not var.peek: + cast = f"({cast_type})" if var.type else "" + bits = ".bits" if cast and not extract_bits else "" + if var.name not in UNUSED and not var.is_array(): + if var.condition: + if var.condition == "0": + continue + elif var.condition != "1": + out.emit(f"if ({var.condition}) ") + if var == to_write_var: + txt = ( + f"stack_pointer[{self.base_offset.to_c()}]{bits} = {cast}{var.name};\n" + ) + self.base_offset.push(var) + for var in self.variables: + self.base_offset.pop(var) + return txt + def as_comment(self) -> str: return f"/* Variables: {[v.name for v in self.variables]}. Base offset: {self.base_offset.to_c()}. Top offset: {self.top_offset.to_c()} */" From 4c0afa1bbb6b7b6af6ecc143cfd4d74975a86597 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 18 Jul 2024 23:55:54 +0800 Subject: [PATCH 21/25] lint --- Python/executor_cases.c.h | 28 +++++++++++----------- Python/generated_cases.c.h | 26 ++++++++++---------- Tools/cases_generator/generators_common.py | 2 +- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index c7f0c99ae71758..43c9603774ce8a 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -173,7 +173,7 @@ _PyStackRef value; oparg = CURRENT_OPARG(); value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; @@ -3069,7 +3069,7 @@ assert(seq); assert(it->it_index < PyList_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; #endif /* Py_GIL_DISABLED */ stack_pointer[0] = next; @@ -3119,7 +3119,7 @@ assert(seq); assert(it->it_index < PyTuple_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; #endif /* Py_GIL_DISABLED */ stack_pointer[0] = next; @@ -3329,7 +3329,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -3353,7 +3353,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -3375,7 +3375,7 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ stack_pointer[-1] = attr; @@ -3394,7 +3394,7 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ stack_pointer[-1] = attr; @@ -3427,7 +3427,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -3542,11 +3542,11 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; #endif /* Py_GIL_DISABLED */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = method; #endif /* Py_GIL_DISABLED */ stack_pointer[-2 - oparg] = method; @@ -3646,11 +3646,11 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; #endif /* Py_GIL_DISABLED */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = func; #endif /* Py_GIL_DISABLED */ PyStackRef_CLOSE(callable); @@ -4902,7 +4902,7 @@ _PyStackRef value; PyObject *ptr = (PyObject *)CURRENT_OPERAND(); value = PyStackRef_FromPyObjectNew(ptr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; @@ -4937,7 +4937,7 @@ _PyStackRef null; PyObject *ptr = (PyObject *)CURRENT_OPERAND(); value = PyStackRef_FromPyObjectNew(ptr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; #endif /* Py_GIL_DISABLED */ null = PyStackRef_NULL; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 33003e80909a09..7128907f4a9858 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -992,11 +992,11 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; #endif /* Py_GIL_DISABLED */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = func; #endif /* Py_GIL_DISABLED */ PyStackRef_CLOSE(callable); @@ -1107,11 +1107,11 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; #endif /* Py_GIL_DISABLED */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = method; #endif /* Py_GIL_DISABLED */ stack_pointer[-2 - oparg] = method; @@ -3189,7 +3189,7 @@ assert(seq); assert(it->it_index < PyList_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; #endif /* Py_GIL_DISABLED */ } @@ -3285,7 +3285,7 @@ assert(seq); assert(it->it_index < PyTuple_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; #endif /* Py_GIL_DISABLED */ } @@ -4254,7 +4254,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -4293,7 +4293,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -4344,7 +4344,7 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ self = owner; @@ -4425,7 +4425,7 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ } @@ -4470,7 +4470,7 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; #endif /* Py_GIL_DISABLED */ } @@ -4654,7 +4654,7 @@ INSTRUCTION_STATS(LOAD_CONST); _PyStackRef value; value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; #endif /* Py_GIL_DISABLED */ stack_pointer[0] = value; @@ -5672,7 +5672,7 @@ // _LOAD_CONST { value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); - #ifdef Py_GIL_DISABLED /* flush specials */ + #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; #endif /* Py_GIL_DISABLED */ } diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 165a82dcc0ccfd..1e7dbf782ae873 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -220,7 +220,7 @@ def emit_tokens( txt = stack.write_variable_to_stack(out, var_to_flush) if txt: out.start_line() - out.emit("#ifdef Py_GIL_DISABLED /* flush specials */ \n") + out.emit("#ifdef Py_GIL_DISABLED /* flush specials */\n") out.emit(txt) out.emit("#endif /* Py_GIL_DISABLED */\n") var_to_flush = None From 71aed5c725fbba86ad320c91861e141083d39c47 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 19 Jul 2024 02:56:52 +0800 Subject: [PATCH 22/25] Simpler implementation --- Python/bytecodes.c | 18 ++--- Python/executor_cases.c.h | 92 ++++++++++++++-------- Python/generated_cases.c.h | 83 ++++++++++++------- Python/optimizer_cases.c.h | 16 ++-- Tools/cases_generator/analyzer.py | 23 ------ Tools/cases_generator/generators_common.py | 44 ++++++++--- Tools/cases_generator/stack.py | 15 +--- 7 files changed, 162 insertions(+), 129 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 16d79753045432..4b2555ed6f09e9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1276,13 +1276,13 @@ dummy_func( } } - tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value[1])) { + tier1 inst(CLEANUP_THROW, (sub_iter_st, last_sent_val_st, exc_value_st -- none, value)) { PyObject *exc_value = PyStackRef_AsPyObjectBorrow(exc_value_st); assert(throwflag); assert(exc_value && PyExceptionInstance_Check(exc_value)); if (PyErr_GivenExceptionMatches(exc_value, PyExc_StopIteration)) { - *value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); DECREF_INPUTS(); none = PyStackRef_None; } @@ -1384,14 +1384,14 @@ dummy_func( macro(UNPACK_SEQUENCE) = _SPECIALIZE_UNPACK_SEQUENCE + _UNPACK_SEQUENCE; - inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1[1], val0[1])) { + inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- val1, val0)) { assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); DEOPT_IF(!PyTuple_CheckExact(seq_o)); DEOPT_IF(PyTuple_GET_SIZE(seq_o) != 2); STAT_INC(UNPACK_SEQUENCE, hit); - *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); DECREF_INPUTS(); } @@ -2216,7 +2216,7 @@ dummy_func( _LOAD_ATTR_WITH_HINT + unused/5; - split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr[1], null if (oparg & 1))) { + split op(_LOAD_ATTR_SLOT, (index/1, owner -- attr, null if (oparg & 1))) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2224,7 +2224,7 @@ dummy_func( DEOPT_IF(attr_o == NULL); STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - *attr = PyStackRef_FromPyObjectNew(attr_o); + attr = PyStackRef_FromPyObjectNew(attr_o); DECREF_INPUTS(); } @@ -2243,10 +2243,10 @@ dummy_func( } - split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr[1], null if (oparg & 1))) { + split op(_LOAD_ATTR_CLASS, (descr/4, owner -- attr, null if (oparg & 1))) { STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - *attr = PyStackRef_FromPyObjectNew(descr); + attr = PyStackRef_FromPyObjectNew(descr); null = PyStackRef_NULL; DECREF_INPUTS(); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 43c9603774ce8a..2d5aba3bb311b9 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -175,7 +175,7 @@ value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -1386,12 +1386,10 @@ case _UNPACK_SEQUENCE_TWO_TUPLE: { _PyStackRef seq; - _PyStackRef *val1; - _PyStackRef *val0; + _PyStackRef val1; + _PyStackRef val0; oparg = CURRENT_OPARG(); seq = stack_pointer[-1]; - val1 = &stack_pointer[-1]; - val0 = &stack_pointer[0]; assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); if (!PyTuple_CheckExact(seq_o)) { @@ -1403,9 +1401,17 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(UNPACK_SEQUENCE, hit); - *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = val0; + #endif /* flush specials */ + val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = val1; + #endif /* flush specials */ PyStackRef_CLOSE(seq); + stack_pointer[-1] = val1; + stack_pointer[0] = val0; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; @@ -1430,6 +1436,8 @@ PyObject **items = _PyTuple_ITEMS(seq_o); for (int i = oparg; --i >= 0; ) { *values++ = PyStackRef_FromPyObjectNew(items[i]); + #ifdef Py_GIL_DISABLED /* flush specials */ + #endif /* flush specials */ } PyStackRef_CLOSE(seq); stack_pointer += -1 + oparg; @@ -1456,6 +1464,8 @@ PyObject **items = _PyList_ITEMS(seq_o); for (int i = oparg; --i >= 0; ) { *values++ = PyStackRef_FromPyObjectNew(items[i]); + #ifdef Py_GIL_DISABLED /* flush specials */ + #endif /* flush specials */ } PyStackRef_CLOSE(seq); stack_pointer += -1 + oparg; @@ -1544,7 +1554,11 @@ "no locals found"); if (true) JUMP_TO_ERROR(); } - locals = PyStackRef_FromPyObjectNew(l);; + locals = PyStackRef_FromPyObjectNew(l); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = locals; + #endif /* flush specials */ + ; stack_pointer[0] = locals; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -2350,11 +2364,10 @@ case _LOAD_ATTR_SLOT_0: { _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; - attr = &stack_pointer[-1]; uint16_t index = (uint16_t)CURRENT_OPERAND(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2365,18 +2378,21 @@ } STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - *attr = PyStackRef_FromPyObjectNew(attr_o); + attr = PyStackRef_FromPyObjectNew(attr_o); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ PyStackRef_CLOSE(owner); + stack_pointer[-1] = attr; break; } case _LOAD_ATTR_SLOT_1: { _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; - attr = &stack_pointer[-1]; uint16_t index = (uint16_t)CURRENT_OPERAND(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -2387,8 +2403,12 @@ } STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - *attr = PyStackRef_FromPyObjectNew(attr_o); + attr = PyStackRef_FromPyObjectNew(attr_o); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ PyStackRef_CLOSE(owner); + stack_pointer[-1] = attr; stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -2416,33 +2436,39 @@ case _LOAD_ATTR_CLASS_0: { _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; - attr = &stack_pointer[-1]; PyObject *descr = (PyObject *)CURRENT_OPERAND(); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - *attr = PyStackRef_FromPyObjectNew(descr); + attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); + stack_pointer[-1] = attr; break; } case _LOAD_ATTR_CLASS_1: { _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; (void)null; owner = stack_pointer[-1]; - attr = &stack_pointer[-1]; PyObject *descr = (PyObject *)CURRENT_OPERAND(); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - *attr = PyStackRef_FromPyObjectNew(descr); + attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); + stack_pointer[-1] = attr; stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3071,7 +3097,7 @@ next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3121,7 +3147,7 @@ next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3331,7 +3357,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3355,7 +3381,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3377,7 +3403,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[-1] = attr; break; } @@ -3396,7 +3422,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[-1] = attr; break; } @@ -3429,7 +3455,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; stack_pointer[-1] = attr; stack_pointer[0] = self; @@ -3544,11 +3570,11 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = method; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); @@ -3648,11 +3674,11 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = func; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ PyStackRef_CLOSE(callable); stack_pointer[-2 - oparg] = func; stack_pointer[-1 - oparg] = self; @@ -4904,7 +4930,7 @@ value = PyStackRef_FromPyObjectNew(ptr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4939,7 +4965,7 @@ value = PyStackRef_FromPyObjectNew(ptr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ null = PyStackRef_NULL; stack_pointer[0] = value; stack_pointer[1] = null; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7128907f4a9858..4afcee4b40a336 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -994,11 +994,11 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = func; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ PyStackRef_CLOSE(callable); } // flush @@ -1109,11 +1109,11 @@ self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1 - oparg] = self; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-2 - oparg] = method; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); @@ -2388,16 +2388,18 @@ _PyStackRef last_sent_val_st; _PyStackRef exc_value_st; _PyStackRef none; - _PyStackRef *value; + _PyStackRef value; exc_value_st = stack_pointer[-1]; last_sent_val_st = stack_pointer[-2]; sub_iter_st = stack_pointer[-3]; - value = &stack_pointer[-2]; PyObject *exc_value = PyStackRef_AsPyObjectBorrow(exc_value_st); assert(throwflag); assert(exc_value && PyExceptionInstance_Check(exc_value)); if (PyErr_GivenExceptionMatches(exc_value, PyExc_StopIteration)) { - *value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-2] = value; + #endif /* flush specials */ PyStackRef_CLOSE(sub_iter_st); PyStackRef_CLOSE(last_sent_val_st); PyStackRef_CLOSE(exc_value_st); @@ -2409,6 +2411,7 @@ goto exception_unwind; } stack_pointer[-3] = none; + stack_pointer[-2] = value; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); @@ -3191,7 +3194,7 @@ next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ } stack_pointer[0] = next; stack_pointer += 1; @@ -3287,7 +3290,7 @@ next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = next; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ } stack_pointer[0] = next; stack_pointer += 1; @@ -4111,7 +4114,7 @@ INSTRUCTION_STATS(LOAD_ATTR_CLASS); static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 9, "incorrect cache size"); _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; /* Skip 1 cache entry */ // _CHECK_ATTR_CLASS @@ -4126,14 +4129,17 @@ /* Skip 2 cache entries */ // _LOAD_ATTR_CLASS { - attr = &stack_pointer[-1]; PyObject *descr = read_obj(&this_instr[6].cache); STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); - *attr = PyStackRef_FromPyObjectNew(descr); - if (oparg & 1) null = PyStackRef_NULL; + attr = PyStackRef_FromPyObjectNew(descr); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ + null = PyStackRef_NULL; PyStackRef_CLOSE(owner); } + stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -4256,7 +4262,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; } stack_pointer[-1] = attr; @@ -4295,7 +4301,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; } stack_pointer[-1] = attr; @@ -4346,7 +4352,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ self = owner; } stack_pointer[-1] = attr; @@ -4427,7 +4433,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ } stack_pointer[-1] = attr; DISPATCH(); @@ -4472,7 +4478,7 @@ attr = PyStackRef_FromPyObjectNew(descr); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[-1] = attr; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ } stack_pointer[-1] = attr; DISPATCH(); @@ -4518,7 +4524,7 @@ INSTRUCTION_STATS(LOAD_ATTR_SLOT); static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 9, "incorrect cache size"); _PyStackRef owner; - _PyStackRef *attr; + _PyStackRef attr; _PyStackRef null = PyStackRef_NULL; /* Skip 1 cache entry */ // _GUARD_TYPE_VERSION @@ -4531,7 +4537,6 @@ } // _LOAD_ATTR_SLOT { - attr = &stack_pointer[-1]; uint16_t index = read_u16(&this_instr[4].cache); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); char *addr = (char *)owner_o + index; @@ -4539,10 +4544,14 @@ DEOPT_IF(attr_o == NULL, LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; - *attr = PyStackRef_FromPyObjectNew(attr_o); + attr = PyStackRef_FromPyObjectNew(attr_o); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = attr; + #endif /* flush specials */ PyStackRef_CLOSE(owner); } /* Skip 5 cache entries */ + stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -4656,7 +4665,7 @@ value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4988,7 +4997,11 @@ "no locals found"); if (true) goto error; } - locals = PyStackRef_FromPyObjectNew(l);; + locals = PyStackRef_FromPyObjectNew(l); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = locals; + #endif /* flush specials */ + ; stack_pointer[0] = locals; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5674,7 +5687,7 @@ value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); #ifdef Py_GIL_DISABLED /* flush specials */ stack_pointer[0] = value; - #endif /* Py_GIL_DISABLED */ + #endif /* flush specials */ } // _RETURN_VALUE retval = value; @@ -6667,6 +6680,8 @@ PyObject **items = _PyList_ITEMS(seq_o); for (int i = oparg; --i >= 0; ) { *values++ = PyStackRef_FromPyObjectNew(items[i]); + #ifdef Py_GIL_DISABLED /* flush specials */ + #endif /* flush specials */ } PyStackRef_CLOSE(seq); stack_pointer += -1 + oparg; @@ -6691,6 +6706,8 @@ PyObject **items = _PyTuple_ITEMS(seq_o); for (int i = oparg; --i >= 0; ) { *values++ = PyStackRef_FromPyObjectNew(items[i]); + #ifdef Py_GIL_DISABLED /* flush specials */ + #endif /* flush specials */ } PyStackRef_CLOSE(seq); stack_pointer += -1 + oparg; @@ -6704,20 +6721,26 @@ INSTRUCTION_STATS(UNPACK_SEQUENCE_TWO_TUPLE); static_assert(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE == 1, "incorrect cache size"); _PyStackRef seq; - _PyStackRef *val1; - _PyStackRef *val0; + _PyStackRef val1; + _PyStackRef val0; /* Skip 1 cache entry */ seq = stack_pointer[-1]; - val1 = &stack_pointer[-1]; - val0 = &stack_pointer[0]; assert(oparg == 2); PyObject *seq_o = PyStackRef_AsPyObjectBorrow(seq); DEOPT_IF(!PyTuple_CheckExact(seq_o), UNPACK_SEQUENCE); DEOPT_IF(PyTuple_GET_SIZE(seq_o) != 2, UNPACK_SEQUENCE); STAT_INC(UNPACK_SEQUENCE, hit); - *val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); - *val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[0] = val0; + #endif /* flush specials */ + val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); + #ifdef Py_GIL_DISABLED /* flush specials */ + stack_pointer[-1] = val1; + #endif /* flush specials */ PyStackRef_CLOSE(seq); + stack_pointer[-1] = val1; + stack_pointer[0] = val0; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 820ddce569d479..978aa911b52efc 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -705,16 +705,12 @@ } case _UNPACK_SEQUENCE_TWO_TUPLE: { - _Py_UopsSymbol **val1; - _Py_UopsSymbol **val0; - val1 = &stack_pointer[-1]; - val0 = &stack_pointer[0]; - for (int _i = 1; --_i >= 0;) { - val1[_i] = sym_new_not_null(ctx); - } - for (int _i = 1; --_i >= 0;) { - val0[_i] = sym_new_not_null(ctx); - } + _Py_UopsSymbol *val1; + _Py_UopsSymbol *val0; + val1 = sym_new_not_null(ctx); + val0 = sym_new_not_null(ctx); + stack_pointer[-1] = val1; + stack_pointer[0] = val0; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 9b145b3fb9b7eb..940504d3d73b2e 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -156,7 +156,6 @@ class Uop: stack: StackEffect caches: list[CacheEntry] body: list[lexer.Token] - token_requires_flush: list[bool] # bitmap to body properties: Properties _size: int = -1 implicitly_created: bool = False @@ -345,25 +344,6 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]: ) return [CacheEntry(i.name, int(i.size)) for i in caches] - -def analyze_specials_requiring_flush(name: str, op: list[lexer.Token], outputs: list[StackItem]) -> list[bool]: - res = [] - for idx, token in enumerate(op): - if token.kind == "IDENTIFIER" and token.text == "PyStackRef_FromPyObjectNew": - should_flush = True - assert op[idx-1].kind == "EQUALS", (f"{name} Result of a specials that is flushed" - " must be written to a stack variable directly") - # Scan to look for the first thing it's assigned to - for assgn_target in reversed(op[:idx-1]): - out_names = [out.name for out in outputs] - if assgn_target.kind == "IDENTIFIER": - should_flush = assgn_target.text in out_names - break - res.append(should_flush) - else: - res.append(False) - return res - def variable_used(node: parser.InstDef, name: str) -> bool: """Determine whether a variable with a given name is used in a node.""" return any( @@ -646,7 +626,6 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, - token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=compute_properties(op), ) if effect_depends_on_oparg_1(op) and "split" in op.annotations: @@ -665,7 +644,6 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, - token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=properties, ) rep.replicates = result @@ -689,7 +667,6 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo stack=stack, caches=analyze_caches(inputs), body=op.block.tokens, - token_requires_flush=analyze_specials_requiring_flush(name, op.block.tokens, stack.outputs), properties=properties, ) rep.replicates = result diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 1e7dbf782ae873..4517fcf31c1429 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -177,6 +177,35 @@ def replace_check_eval_breaker( if not uop.properties.ends_with_eval_breaker: out.emit_at("CHECK_EVAL_BREAKER();", tkn) +def replace_pystackref_frompyobjectnew( + out: CWriter, + tkn: Token, + tkn_iter: Iterator[Token], + uop: Uop, + stack: Stack, + inst: Instruction | None, +) -> None: + out.emit(tkn) + emit_to(out, tkn_iter, "SEMI") + out.emit(";\n") + target = uop.body.index(tkn) + assert uop.body[target-1].kind == "EQUALS", (f"{uop.name} Result of a specials that is flushed" + " must be written to a stack variable directly") + # Scan to look for the first thing it's assigned to + found_valid_assignment = "" + for assgn_target in reversed(uop.body[:target-1]): + out_names = [out.name for out in uop.stack.outputs] + if assgn_target.kind == "IDENTIFIER": + if assgn_target.text in out_names: + found_valid_assignment = assgn_target.text + break + + if found_valid_assignment: + out.start_line() + out.emit("#ifdef Py_GIL_DISABLED /* flush specials */\n") + stack.write_variable_to_stack(out, found_valid_assignment) + out.emit("#endif /* flush specials */\n") + REPLACEMENT_FUNCTIONS = { "EXIT_IF": replace_deopt, @@ -186,6 +215,7 @@ def replace_check_eval_breaker( "DECREF_INPUTS": replace_decrefs, "CHECK_EVAL_BREAKER": replace_check_eval_breaker, "SYNC_SP": replace_sync_sp, + "PyStackRef_FromPyObjectNew": replace_pystackref_frompyobjectnew, } ReplacementFunctionType = Callable[ @@ -207,23 +237,11 @@ def emit_tokens( return tkn_iter = iter(tkns) out.start_line() - var_to_flush = None - for idx, tkn in enumerate(tkn_iter): + for tkn in tkn_iter: if tkn.kind == "IDENTIFIER" and tkn.text in replacement_functions: replacement_functions[tkn.text](out, tkn, tkn_iter, uop, stack, inst) else: out.emit(tkn) - if uop.token_requires_flush[idx]: - assert tkns[idx-3].kind == "IDENTIFIER" - var_to_flush = tkns[idx-3].text - if var_to_flush and tkn.kind == "SEMI": - txt = stack.write_variable_to_stack(out, var_to_flush) - if txt: - out.start_line() - out.emit("#ifdef Py_GIL_DISABLED /* flush specials */\n") - out.emit(txt) - out.emit("#endif /* Py_GIL_DISABLED */\n") - var_to_flush = None def cflags(p: Properties) -> str: diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index 1e1aca6ce14699..cc484c73fe7720 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -213,30 +213,23 @@ def flush(self, out: CWriter, cast_type: str = "uintptr_t", extract_bits: bool = self.peek_offset.clear() out.start_line() - def write_variable_to_stack(self, out: CWriter, var: str, cast_type: str = "uintptr_t", extract_bits: bool = False) -> str: + def write_variable_to_stack(self, out: CWriter, var_name: str, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: out.start_line() - var = [variable for variable in self.variables if variable.name == var] - assert len(var) == 1 + var = [variable for variable in self.variables if variable.name == var_name] + assert len(var) == 1, f"{var_name} not found" to_write_var = var[0] - txt = "" for var in self.variables: if not var.peek: cast = f"({cast_type})" if var.type else "" bits = ".bits" if cast and not extract_bits else "" if var.name not in UNUSED and not var.is_array(): - if var.condition: - if var.condition == "0": - continue - elif var.condition != "1": - out.emit(f"if ({var.condition}) ") if var == to_write_var: - txt = ( + out.emit( f"stack_pointer[{self.base_offset.to_c()}]{bits} = {cast}{var.name};\n" ) self.base_offset.push(var) for var in self.variables: self.base_offset.pop(var) - return txt def as_comment(self) -> str: return f"/* Variables: {[v.name for v in self.variables]}. Base offset: {self.base_offset.to_c()}. Top offset: {self.top_offset.to_c()} */" From 1d19a9743bc0f1d585491ff8f375693ba1430b1e Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 19 Jul 2024 02:58:40 +0800 Subject: [PATCH 23/25] reduce diff --- Python/executor_cases.c.h | 21 +++++++++++++++------ Tools/cases_generator/analyzer.py | 9 +++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 2d5aba3bb311b9..3168927e2a9b69 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -2224,6 +2224,9 @@ attr = PyStackRef_FromPyObjectSteal(attr_o); PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); + assert(WITHIN_STACK_BOUNDS()); break; } @@ -2246,8 +2249,8 @@ attr = PyStackRef_FromPyObjectSteal(attr_o); PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - stack_pointer[0] = null; - stack_pointer += 1; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); break; } @@ -2384,6 +2387,9 @@ #endif /* flush specials */ PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); + assert(WITHIN_STACK_BOUNDS()); break; } @@ -2409,8 +2415,8 @@ #endif /* flush specials */ PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - stack_pointer[0] = null; - stack_pointer += 1; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); break; } @@ -2450,6 +2456,9 @@ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); + assert(WITHIN_STACK_BOUNDS()); break; } @@ -2469,8 +2478,8 @@ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - stack_pointer[0] = null; - stack_pointer += 1; + if (oparg & 1) stack_pointer[0] = null; + stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); break; } diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 940504d3d73b2e..9e31d0c7d70c0d 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -618,12 +618,11 @@ def compute_properties(op: parser.InstDef) -> Properties: def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uops: dict[str, Uop]) -> Uop: - stack = analyze_stack(op) result = Uop( name=name, context=op.context, annotations=op.annotations, - stack=stack, + stack=analyze_stack(op), caches=analyze_caches(inputs), body=op.block.tokens, properties=compute_properties(op), @@ -636,12 +635,11 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo if properties.oparg: # May not need oparg anymore properties.oparg = any(token.text == "oparg" for token in op.block.tokens) - stack = analyze_stack(op, bit) rep = Uop( name=name_x, context=op.context, annotations=op.annotations, - stack=stack, + stack=analyze_stack(op), caches=analyze_caches(inputs), body=op.block.tokens, properties=properties, @@ -659,12 +657,11 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo properties = compute_properties(op) properties.oparg = False properties.const_oparg = oparg - stack = analyze_stack(op) rep = Uop( name=name_x, context=op.context, annotations=op.annotations, - stack=stack, + stack=analyze_stack(op), caches=analyze_caches(inputs), body=op.block.tokens, properties=properties, From 2b383b1114e6f98f78fb631dc43ebdb8c1bd29fc Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 19 Jul 2024 02:59:59 +0800 Subject: [PATCH 24/25] reduce diff --- Python/executor_cases.c.h | 21 ++++++--------------- Tools/cases_generator/analyzer.py | 3 ++- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 3168927e2a9b69..2d5aba3bb311b9 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -2224,9 +2224,6 @@ attr = PyStackRef_FromPyObjectSteal(attr_o); PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); - assert(WITHIN_STACK_BOUNDS()); break; } @@ -2249,8 +2246,8 @@ attr = PyStackRef_FromPyObjectSteal(attr_o); PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); + stack_pointer[0] = null; + stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; } @@ -2387,9 +2384,6 @@ #endif /* flush specials */ PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); - assert(WITHIN_STACK_BOUNDS()); break; } @@ -2415,8 +2409,8 @@ #endif /* flush specials */ PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); + stack_pointer[0] = null; + stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; } @@ -2456,9 +2450,6 @@ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); - assert(WITHIN_STACK_BOUNDS()); break; } @@ -2478,8 +2469,8 @@ null = PyStackRef_NULL; PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; - if (oparg & 1) stack_pointer[0] = null; - stack_pointer += (oparg & 1); + stack_pointer[0] = null; + stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; } diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 9e31d0c7d70c0d..7776b961789f26 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -344,6 +344,7 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]: ) return [CacheEntry(i.name, int(i.size)) for i in caches] + def variable_used(node: parser.InstDef, name: str) -> bool: """Determine whether a variable with a given name is used in a node.""" return any( @@ -639,7 +640,7 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo name=name_x, context=op.context, annotations=op.annotations, - stack=analyze_stack(op), + stack=analyze_stack(op, bit), caches=analyze_caches(inputs), body=op.block.tokens, properties=properties, From 2d2cd7c11b6f669607252f62ed1aba5bec08126f Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 19 Jul 2024 03:00:39 +0800 Subject: [PATCH 25/25] reduce diff again (sorry) --- Python/bytecodes.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 4b2555ed6f09e9..bc4f57df2e8e01 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -3476,7 +3476,6 @@ dummy_func( assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - stack_pointer[-2 - oparg] = method; assert(PyFunction_Check(PyStackRef_AsPyObjectBorrow(method))); PyStackRef_CLOSE(callable); }