From 66f18c7b26d954912a845f9a38a5b463cd4019f7 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 8 Oct 2024 11:34:27 +0100 Subject: [PATCH 1/3] Tidy up pycore_stackref.h, splitting into gil and no-gil sections --- Include/internal/pycore_stackref.h | 156 +++++++++++------------------ 1 file changed, 59 insertions(+), 97 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index cf6dd22cfb18d1..11977669bb2419 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -60,54 +60,21 @@ typedef union _PyStackRef { #define Py_TAG_BITS ((uintptr_t)1) #ifdef Py_GIL_DISABLED - static const _PyStackRef PyStackRef_NULL = { .bits = 0 | Py_TAG_DEFERRED}; -#else - static const _PyStackRef PyStackRef_NULL = { .bits = 0 }; -#endif - -#define PyStackRef_IsNull(stackref) ((stackref).bits == PyStackRef_NULL.bits) - - -#ifdef Py_GIL_DISABLED + static const _PyStackRef PyStackRef_NULL = { .bits = Py_TAG_DEFERRED}; +# define PyStackRef_IsNull(stackref) ((stackref).bits == Py_TAG_DEFERRED) # define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) -#else -# define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) }) -#endif - -#ifdef Py_GIL_DISABLED # define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) -#else -# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) }) -#endif - -#ifdef Py_GIL_DISABLED # define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) | Py_TAG_DEFERRED }) -#else -# define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) }) -#endif - -// Note: the following are all macros because MSVC (Windows) has trouble inlining them. -#define PyStackRef_Is(a, b) ((a).bits == (b).bits) - -#define PyStackRef_IsDeferred(ref) (((ref).bits & Py_TAG_BITS) == Py_TAG_DEFERRED) - - -#ifdef Py_GIL_DISABLED -// Gets a PyObject * from a _PyStackRef static inline PyObject * PyStackRef_AsPyObjectBorrow(_PyStackRef stackref) { PyObject *cleared = ((PyObject *)((stackref).bits & (~Py_TAG_BITS))); return cleared; } -#else -# define PyStackRef_AsPyObjectBorrow(stackref) ((PyObject *)(stackref).bits) -#endif -// Converts a PyStackRef back to a PyObject *, stealing the -// PyStackRef. -#ifdef Py_GIL_DISABLED +#define PyStackRef_IsDeferred(ref) (((ref).bits & Py_TAG_BITS) == Py_TAG_DEFERRED) + static inline PyObject * PyStackRef_AsPyObjectSteal(_PyStackRef stackref) { @@ -117,18 +84,7 @@ PyStackRef_AsPyObjectSteal(_PyStackRef stackref) } return PyStackRef_AsPyObjectBorrow(stackref); } -#else -# define PyStackRef_AsPyObjectSteal(stackref) PyStackRef_AsPyObjectBorrow(stackref) -#endif - -// Converts a PyStackRef back to a PyObject *, converting the -// stackref to a new reference. -#define PyStackRef_AsPyObjectNew(stackref) Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref)) - -#define PyStackRef_TYPE(stackref) Py_TYPE(PyStackRef_AsPyObjectBorrow(stackref)) -// Converts a PyObject * to a PyStackRef, stealing the reference -#ifdef Py_GIL_DISABLED static inline _PyStackRef _PyStackRef_FromPyObjectSteal(PyObject *obj) { @@ -139,13 +95,7 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj) return ((_PyStackRef){.bits = ((uintptr_t)(obj)) | tag}); } # define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj)) -#else -# define PyStackRef_FromPyObjectSteal(obj) ((_PyStackRef){.bits = ((uintptr_t)(obj))}) -#endif - -// Converts a PyObject * to a PyStackRef, with a new reference -#ifdef Py_GIL_DISABLED static inline _PyStackRef PyStackRef_FromPyObjectNew(PyObject *obj) { @@ -160,12 +110,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj) } } # define PyStackRef_FromPyObjectNew(obj) PyStackRef_FromPyObjectNew(_PyObject_CAST(obj)) -#else -# define PyStackRef_FromPyObjectNew(obj) ((_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) }) -#endif -#ifdef Py_GIL_DISABLED -// Same as PyStackRef_FromPyObjectNew but only for immortal objects. static inline _PyStackRef PyStackRef_FromPyObjectImmortal(PyObject *obj) { @@ -176,22 +121,7 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; } # define PyStackRef_FromPyObjectImmortal(obj) PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj)) -#else -# define PyStackRef_FromPyObjectImmortal(obj) ((_PyStackRef){ .bits = (uintptr_t)(obj) }) -#endif - -#define PyStackRef_CLEAR(op) \ - do { \ - _PyStackRef *_tmp_op_ptr = &(op); \ - _PyStackRef _tmp_old_op = (*_tmp_op_ptr); \ - if (!PyStackRef_IsNull(_tmp_old_op)) { \ - *_tmp_op_ptr = PyStackRef_NULL; \ - PyStackRef_CLOSE(_tmp_old_op); \ - } \ - } while (0) - -#ifdef Py_GIL_DISABLED # define PyStackRef_CLOSE(REF) \ do { \ _PyStackRef _close_tmp = (REF); \ @@ -200,20 +130,7 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) Py_DECREF(PyStackRef_AsPyObjectBorrow(_close_tmp)); \ } \ } while (0) -#else -# define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)) -#endif -#define PyStackRef_XCLOSE(stackref) \ - do { \ - _PyStackRef _tmp = (stackref); \ - if (!PyStackRef_IsNull(_tmp)) { \ - PyStackRef_CLOSE(_tmp); \ - } \ - } while (0); - - -#ifdef Py_GIL_DISABLED static inline _PyStackRef PyStackRef_DUP(_PyStackRef stackref) { @@ -227,9 +144,6 @@ PyStackRef_DUP(_PyStackRef stackref) Py_INCREF(PyStackRef_AsPyObjectBorrow(stackref)); return stackref; } -#else -# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))) -#endif // Convert a possibly deferred reference to a strong reference. static inline _PyStackRef @@ -238,13 +152,61 @@ PyStackRef_AsStrongReference(_PyStackRef stackref) return PyStackRef_FromPyObjectSteal(PyStackRef_AsPyObjectSteal(stackref)); } -static inline void -_PyObjectStack_FromStackRefStack(PyObject **dst, const _PyStackRef *src, size_t length) -{ - for (size_t i = 0; i < length; i++) { - dst[i] = PyStackRef_AsPyObjectBorrow(src[i]); - } -} + +#else // Py_GIL_DISABLED + // With GIL + static const _PyStackRef PyStackRef_NULL = { .bits = 0 }; +# define PyStackRef_IsNull(stackref) ((stackref).bits == 0) +# define PyStackRef_True ((_PyStackRef){.bits = (uintptr_t)&_Py_TrueStruct }) +# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) }) +# define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) }) + +# define PyStackRef_AsPyObjectBorrow(stackref) ((PyObject *)(stackref).bits) + +# define PyStackRef_AsPyObjectSteal(stackref) PyStackRef_AsPyObjectBorrow(stackref) + +# define PyStackRef_FromPyObjectSteal(obj) ((_PyStackRef){.bits = ((uintptr_t)(obj))}) + +# define PyStackRef_FromPyObjectNew(obj) ((_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) }) + +# define PyStackRef_FromPyObjectImmortal(obj) ((_PyStackRef){ .bits = (uintptr_t)(obj) }) + +# define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)) + +# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))) + + +#endif // Py_GIL_DISABLED + +// Note: this is a macro because MSVC (Windows) has trouble inlining it. + +#define PyStackRef_Is(a, b) ((a).bits == (b).bits) + +// Converts a PyStackRef back to a PyObject *, converting the +// stackref to a new reference. +#define PyStackRef_AsPyObjectNew(stackref) Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref)) + +#define PyStackRef_TYPE(stackref) Py_TYPE(PyStackRef_AsPyObjectBorrow(stackref)) + +#define PyStackRef_CLEAR(op) \ + do { \ + _PyStackRef *_tmp_op_ptr = &(op); \ + _PyStackRef _tmp_old_op = (*_tmp_op_ptr); \ + if (!PyStackRef_IsNull(_tmp_old_op)) { \ + *_tmp_op_ptr = PyStackRef_NULL; \ + PyStackRef_CLOSE(_tmp_old_op); \ + } \ + } while (0) + +#define PyStackRef_XCLOSE(stackref) \ + do { \ + _PyStackRef _tmp = (stackref); \ + if (!PyStackRef_IsNull(_tmp)) { \ + PyStackRef_CLOSE(_tmp); \ + } \ + } while (0); + + // StackRef type checks From 89e6d1f6df6f09f95670aca2ca40ea892c8921c4 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 9 Oct 2024 11:42:36 +0100 Subject: [PATCH 2/3] Use conventional indentation --- Include/internal/pycore_stackref.h | 44 ++++++++++++++++-------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 11977669bb2419..9b5a45958f84d5 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -60,11 +60,12 @@ typedef union _PyStackRef { #define Py_TAG_BITS ((uintptr_t)1) #ifdef Py_GIL_DISABLED - static const _PyStackRef PyStackRef_NULL = { .bits = Py_TAG_DEFERRED}; -# define PyStackRef_IsNull(stackref) ((stackref).bits == Py_TAG_DEFERRED) -# define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) -# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) -# define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) | Py_TAG_DEFERRED }) + +static const _PyStackRef PyStackRef_NULL = { .bits = Py_TAG_DEFERRED}; +#define PyStackRef_IsNull(stackref) ((stackref).bits == Py_TAG_DEFERRED) +#define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) +#define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) +#define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) | Py_TAG_DEFERRED }) static inline PyObject * PyStackRef_AsPyObjectBorrow(_PyStackRef stackref) @@ -109,7 +110,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj) return (_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) | Py_TAG_PTR }; } } -# define PyStackRef_FromPyObjectNew(obj) PyStackRef_FromPyObjectNew(_PyObject_CAST(obj)) +#define PyStackRef_FromPyObjectNew(obj) PyStackRef_FromPyObjectNew(_PyObject_CAST(obj)) static inline _PyStackRef PyStackRef_FromPyObjectImmortal(PyObject *obj) @@ -120,9 +121,9 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) assert(_Py_IsImmortal(obj)); return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; } -# define PyStackRef_FromPyObjectImmortal(obj) PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj)) +#define PyStackRef_FromPyObjectImmortal(obj) PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj)) -# define PyStackRef_CLOSE(REF) \ +#define PyStackRef_CLOSE(REF) \ do { \ _PyStackRef _close_tmp = (REF); \ assert(!PyStackRef_IsNull(_close_tmp)); \ @@ -154,26 +155,27 @@ PyStackRef_AsStrongReference(_PyStackRef stackref) #else // Py_GIL_DISABLED - // With GIL - static const _PyStackRef PyStackRef_NULL = { .bits = 0 }; -# define PyStackRef_IsNull(stackref) ((stackref).bits == 0) -# define PyStackRef_True ((_PyStackRef){.bits = (uintptr_t)&_Py_TrueStruct }) -# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) }) -# define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) }) -# define PyStackRef_AsPyObjectBorrow(stackref) ((PyObject *)(stackref).bits) +// With GIL +static const _PyStackRef PyStackRef_NULL = { .bits = 0 }; +#define PyStackRef_IsNull(stackref) ((stackref).bits == 0) +#define PyStackRef_True ((_PyStackRef){.bits = (uintptr_t)&_Py_TrueStruct }) +#define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) }) +#define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) }) + +#define PyStackRef_AsPyObjectBorrow(stackref) ((PyObject *)(stackref).bits) -# define PyStackRef_AsPyObjectSteal(stackref) PyStackRef_AsPyObjectBorrow(stackref) +#define PyStackRef_AsPyObjectSteal(stackref) PyStackRef_AsPyObjectBorrow(stackref) -# define PyStackRef_FromPyObjectSteal(obj) ((_PyStackRef){.bits = ((uintptr_t)(obj))}) +#define PyStackRef_FromPyObjectSteal(obj) ((_PyStackRef){.bits = ((uintptr_t)(obj))}) -# define PyStackRef_FromPyObjectNew(obj) ((_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) }) +#define PyStackRef_FromPyObjectNew(obj) ((_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) }) -# define PyStackRef_FromPyObjectImmortal(obj) ((_PyStackRef){ .bits = (uintptr_t)(obj) }) +#define PyStackRef_FromPyObjectImmortal(obj) ((_PyStackRef){ .bits = (uintptr_t)(obj) }) -# define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)) +#define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)) -# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))) +#define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))) #endif // Py_GIL_DISABLED From 3c7c46d6cc2ff0b1a8e63109add54e7f34a4327d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 9 Oct 2024 11:44:59 +0100 Subject: [PATCH 3/3] Add review comment --- Include/internal/pycore_stackref.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 9b5a45958f84d5..7d1eb11aa5ecb8 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -62,7 +62,7 @@ typedef union _PyStackRef { #ifdef Py_GIL_DISABLED static const _PyStackRef PyStackRef_NULL = { .bits = Py_TAG_DEFERRED}; -#define PyStackRef_IsNull(stackref) ((stackref).bits == Py_TAG_DEFERRED) +#define PyStackRef_IsNull(stackref) ((stackref).bits == PyStackRef_NULL.bits) #define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) #define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) #define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) | Py_TAG_DEFERRED })