From 0be9d62ca4a2d0937b6bea1f5a80c5b55c6f8088 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 6 Apr 2022 16:15:26 -0700 Subject: [PATCH 01/23] TEMPORARY hacks to show inline decisions --- PCbuild/build.bat | 2 +- PCbuild/pyproject.props | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/PCbuild/build.bat b/PCbuild/build.bat index d333ceabd2e53a..646b0998cd3edc 100644 --- a/PCbuild/build.bat +++ b/PCbuild/build.bat @@ -63,7 +63,7 @@ set parallel=/m set verbose=/nologo /v:m /clp:summary set kill= set do_pgo= -set pgo_job=-m test --pgo +set pgo_job=-c0 :CheckOpts if "%~1"=="-h" goto Usage diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index 8d24393aa1861e..bb538e1411d17a 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -45,7 +45,7 @@ true $(EnableControlFlowGuard) /utf-8 %(AdditionalOptions) - true + /d2inlinelogfull:_PyEval_EvalFrameDefault %(AdditionalOptions) true OnlyExplicitInline From abd911876d235d5034305c8ccd36d05b20d4dabe Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 6 Apr 2022 17:06:09 -0700 Subject: [PATCH 02/23] In ceval.c, redefine some macros for speed In particular, Py_DECREF, Py_XDECREF and Py_IS_TYPE are redefined as macros that completely replace the inline functions of the same name. These three came out in the top four of functions that (in MSVC) somehow weren't inlined. (There was another, _Py_atomic_load_32bit_impl, but it is more difficult.) --- Python/ceval.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index 487e09bc664173..895bfddd1cc2bf 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -45,6 +45,20 @@ # error "ceval.c must be build with Py_BUILD_CORE define for best performance" #endif +#if !defined(Py_DEBUG) +// The MSVC compiler fails to inline these, and they're kind of important. + +#undef Py_DECREF +#define Py_DECREF(arg) { PyObject *op = arg; if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } + +#undef Py_IS_TYPE +#define Py_IS_TYPE(ob, type) ((PyObject *)(ob)->ob_type == (type)) + +#undef Py_XDECREF +#define Py_XDECREF(arg) { PyObject *op1 = arg; if (op1 != NULL) { Py_DECREF(op1); } } + +#endif + /* Forward declarations */ static PyObject *trace_call_function( PyThreadState *tstate, PyObject *callable, PyObject **stack, @@ -1579,6 +1593,7 @@ static inline bool is_method(PyObject **stack_pointer, int args) { return PEEK(args+2) != NULL; } +#define is_method(stack_pointer, args) (PEEK((args)+2) != NULL) #define KWNAMES_LEN() \ (call_shape.kwnames == NULL ? 0 : ((int)PyTuple_GET_SIZE(call_shape.kwnames))) From 50e3d7b2b44a04ac9ea90bcca4a8e9f9c32bd119 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 6 Apr 2022 17:55:46 -0700 Subject: [PATCH 03/23] Victor's review --- Python/ceval.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 895bfddd1cc2bf..c7cd5f6afb3942 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -46,16 +46,17 @@ #endif #if !defined(Py_DEBUG) -// The MSVC compiler fails to inline these, and they're kind of important. +// bpo-45116: The MSVC compiler fails to inline these in PGO build, +// and they're kind of important for performance. #undef Py_DECREF -#define Py_DECREF(arg) { PyObject *op = arg; if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } +#define Py_DECREF(arg) do { PyObject *op = arg; if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } while (0) #undef Py_IS_TYPE #define Py_IS_TYPE(ob, type) ((PyObject *)(ob)->ob_type == (type)) #undef Py_XDECREF -#define Py_XDECREF(arg) { PyObject *op1 = arg; if (op1 != NULL) { Py_DECREF(op1); } } +#define Py_XDECREF(arg) do { PyObject *op1 = arg; if (op1 != NULL) { Py_DECREF(op1); } } while (0) #endif @@ -1589,10 +1590,6 @@ typedef struct { PyObject *kwnames; } CallShape; -static inline bool -is_method(PyObject **stack_pointer, int args) { - return PEEK(args+2) != NULL; -} #define is_method(stack_pointer, args) (PEEK((args)+2) != NULL) #define KWNAMES_LEN() \ From ab6660e4d445101f509051ec5dd4f269247032a6 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 6 Apr 2022 18:43:05 -0700 Subject: [PATCH 04/23] Add _Py_atomic_load_32bit_impl and use cast macro --- Python/ceval.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index ade3fea43127b1..55b7e4fd30dfc3 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -49,13 +49,15 @@ // and they're kind of important for performance. #undef Py_DECREF -#define Py_DECREF(arg) do { PyObject *op = arg; if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } while (0) +#define Py_DECREF(arg) do { PyObject *op = _PyObject_CAST(arg); if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } while (0) #undef Py_IS_TYPE -#define Py_IS_TYPE(ob, type) ((PyObject *)(ob)->ob_type == (type)) +#define Py_IS_TYPE(ob, type) (_PyObject_CAST(ob)->ob_type == (type)) #undef Py_XDECREF -#define Py_XDECREF(arg) do { PyObject *op1 = arg; if (op1 != NULL) { Py_DECREF(op1); } } while (0) +#define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) + +#define _Py_atomic_load_32bit_impl(value, order) (assert((order) == _Py_memory_order_relaxed), *(value)) #endif From ae6b53cc3c2ef33229b887123d8728f8009ad4f7 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 6 Apr 2022 21:16:22 -0700 Subject: [PATCH 05/23] Move _Py_atomic_load_32bit_impl out of '#if' So that the assert() in the macro will be checked in debug mode. --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 55b7e4fd30dfc3..30fe175a4c60c9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -57,10 +57,10 @@ #undef Py_XDECREF #define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) -#define _Py_atomic_load_32bit_impl(value, order) (assert((order) == _Py_memory_order_relaxed), *(value)) - #endif +#define _Py_atomic_load_32bit_impl(value, order) (assert((order) == _Py_memory_order_relaxed), *(value)) + /* Forward declarations */ static PyObject *trace_call_function( PyThreadState *tstate, PyObject *callable, PyObject **stack, From 01cff81590685b9ee688429516aaa630c25cc6d8 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 7 Apr 2022 20:43:11 -0700 Subject: [PATCH 06/23] Go back to full PGO test suite --- PCbuild/build.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PCbuild/build.bat b/PCbuild/build.bat index 646b0998cd3edc..d333ceabd2e53a 100644 --- a/PCbuild/build.bat +++ b/PCbuild/build.bat @@ -63,7 +63,7 @@ set parallel=/m set verbose=/nologo /v:m /clp:summary set kill= set do_pgo= -set pgo_job=-c0 +set pgo_job=-m test --pgo :CheckOpts if "%~1"=="-h" goto Usage From 69da380ab4c46c3878def4156912a1d894ddf2a9 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 7 Apr 2022 21:20:21 -0700 Subject: [PATCH 07/23] Longer comment; improve _Py_atomic_load_relaxed --- Python/ceval.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 30fe175a4c60c9..0d28475073843c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -44,9 +44,12 @@ # error "ceval.c must be build with Py_BUILD_CORE define for best performance" #endif -#if !defined(Py_DEBUG) -// bpo-45116: The MSVC compiler fails to inline these in PGO build, -// and they're kind of important for performance. +#ifndef Py_DEBUG +// bpo-45116: The MSVC compiler does not inline these static inline functions +// in PGO build in _PyEval_EvalFrameDefault(), because this function is over +// the limit of PGO, and that limit cannot be configured. +// Define them as macros to make sure that they are always inlined by the +// preprocessor. #undef Py_DECREF #define Py_DECREF(arg) do { PyObject *op = _PyObject_CAST(arg); if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } while (0) @@ -57,9 +60,13 @@ #undef Py_XDECREF #define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) +#if SIZEOF_INT == 4 +#undef _Py_atomic_load_relaxed +#define _Py_atomic_load_relaxed(ATOMIC_VAL) (*((volatile int*)&((ATOMIC_VAL)->_value))) +#endif + #endif -#define _Py_atomic_load_32bit_impl(value, order) (assert((order) == _Py_memory_order_relaxed), *(value)) /* Forward declarations */ static PyObject *trace_call_function( From 2894f6e62d47b008b908fa946f8a41f57340ce27 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 8 Apr 2022 17:05:24 -0700 Subject: [PATCH 08/23] Only specialize atomic-load-relaxed if MSC --- Python/ceval.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 0d28475073843c..58a4bf60f7cff7 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -60,11 +60,11 @@ #undef Py_XDECREF #define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) -#if SIZEOF_INT == 4 -#undef _Py_atomic_load_relaxed -#define _Py_atomic_load_relaxed(ATOMIC_VAL) (*((volatile int*)&((ATOMIC_VAL)->_value))) #endif +#if defined(_MSC_VER) && SIZEOF_INT == 4 +#undef _Py_atomic_load_relaxed +#define _Py_atomic_load_relaxed(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) #endif From 4b70976b06cf386ddbd759a6019307ab71fbdeaa Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 8 Apr 2022 21:23:42 -0700 Subject: [PATCH 09/23] Try _Py_atomic_load_relaxed_int32 --- Python/ceval.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 58a4bf60f7cff7..a4b044e9108c4c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -63,8 +63,9 @@ #endif #if defined(_MSC_VER) && SIZEOF_INT == 4 -#undef _Py_atomic_load_relaxed -#define _Py_atomic_load_relaxed(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) +#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) +#else +#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) _Py_atomic_load_relaxed(ATOMIC_VAL) #endif @@ -156,10 +157,10 @@ COMPUTE_EVAL_BREAKER(PyInterpreterState *interp, struct _ceval_state *ceval2) { _Py_atomic_store_relaxed(&ceval2->eval_breaker, - _Py_atomic_load_relaxed(&ceval2->gil_drop_request) - | (_Py_atomic_load_relaxed(&ceval->signals_pending) + _Py_atomic_load_relaxed_int32(&ceval2->gil_drop_request) + | (_Py_atomic_load_relaxed_int32(&ceval->signals_pending) && _Py_ThreadCanHandleSignals(interp)) - | (_Py_atomic_load_relaxed(&ceval2->pending.calls_to_do) + | (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do) && _Py_ThreadCanHandlePendingCalls()) | ceval2->pending.async_exc); } @@ -704,7 +705,7 @@ _Py_FinishPendingCalls(PyThreadState *tstate) struct _pending_calls *pending = &tstate->interp->ceval.pending; - if (!_Py_atomic_load_relaxed(&(pending->calls_to_do))) { + if (!_Py_atomic_load_relaxed_int32(&(pending->calls_to_do))) { return; } @@ -1151,7 +1152,7 @@ eval_frame_handle_pending(PyThreadState *tstate) struct _ceval_runtime_state *ceval = &runtime->ceval; /* Pending signals */ - if (_Py_atomic_load_relaxed(&ceval->signals_pending)) { + if (_Py_atomic_load_relaxed_int32(&ceval->signals_pending)) { if (handle_signals(tstate) != 0) { return -1; } @@ -1159,14 +1160,14 @@ eval_frame_handle_pending(PyThreadState *tstate) /* Pending calls */ struct _ceval_state *ceval2 = &tstate->interp->ceval; - if (_Py_atomic_load_relaxed(&ceval2->pending.calls_to_do)) { + if (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do)) { if (make_pending_calls(tstate->interp) != 0) { return -1; } } /* GIL drop request */ - if (_Py_atomic_load_relaxed(&ceval2->gil_drop_request)) { + if (_Py_atomic_load_relaxed_int32(&ceval2->gil_drop_request)) { /* Give another thread a chance */ if (_PyThreadState_Swap(&runtime->gilstate, NULL) != tstate) { Py_FatalError("tstate mix-up"); @@ -1317,7 +1318,7 @@ eval_frame_handle_pending(PyThreadState *tstate) #define CHECK_EVAL_BREAKER() \ _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY(); \ - if (_Py_atomic_load_relaxed(eval_breaker)) { \ + if (_Py_atomic_load_relaxed_int32(eval_breaker)) { \ goto handle_eval_breaker; \ } @@ -1746,7 +1747,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int PREDICTED(RESUME_QUICK); assert(tstate->cframe == &cframe); assert(frame == cframe.current_frame); - if (_Py_atomic_load_relaxed(eval_breaker) && oparg < 2) { + if (_Py_atomic_load_relaxed_int32(eval_breaker) && oparg < 2) { goto handle_eval_breaker; } DISPATCH(); From 5617fcdf08dda8ce87a81f8d8b190715091b4564 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 11 Apr 2022 13:06:39 -0700 Subject: [PATCH 10/23] MSC is better off with the default Py_UNREACHABLE() --- Include/pymacro.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index 2728496976de7e..4e2188379f02af 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -122,8 +122,6 @@ # define Py_UNREACHABLE() __builtin_unreachable() #elif defined(__clang__) || defined(__INTEL_COMPILER) # define Py_UNREACHABLE() __builtin_unreachable() -#elif defined(_MSC_VER) -# define Py_UNREACHABLE() __assume(0) #else # define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") From a901b91efec84230edd6b6a2314623ea20000e59 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 11 Apr 2022 13:07:09 -0700 Subject: [PATCH 11/23] Update bpo numbers to GH issue numbers --- Python/ceval.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index a4b044e9108c4c..bc68ee45ccf240 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -45,7 +45,7 @@ #endif #ifndef Py_DEBUG -// bpo-45116: The MSVC compiler does not inline these static inline functions +// GH-89279: The MSVC compiler does not inline these static inline functions // in PGO build in _PyEval_EvalFrameDefault(), because this function is over // the limit of PGO, and that limit cannot be configured. // Define them as macros to make sure that they are always inlined by the @@ -62,6 +62,7 @@ #endif +// GH-89279: Similar to above, force inlining by using a macro. #if defined(_MSC_VER) && SIZEOF_INT == 4 #define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) #else From cceb531f3d76c454754f34f177519bd5324f3304 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 13 Apr 2022 14:47:23 -0700 Subject: [PATCH 12/23] Split accidentally merged lines --- PCbuild/pyproject.props | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index bb538e1411d17a..9490625ac456bf 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -45,7 +45,8 @@ true $(EnableControlFlowGuard) /utf-8 %(AdditionalOptions) - /d2inlinelogfull:_PyEval_EvalFrameDefault %(AdditionalOptions) true + /d2inlinelogfull:_PyEval_EvalFrameDefault %(AdditionalOptions) + true OnlyExplicitInline From 26e81077e16f5ccee293ca5a8154b74b9fbc7c35 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 13 Apr 2022 14:48:28 -0700 Subject: [PATCH 13/23] Delete temporary /d2inlinelogfull option --- PCbuild/pyproject.props | 1 - 1 file changed, 1 deletion(-) diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index 9490625ac456bf..8d24393aa1861e 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -45,7 +45,6 @@ true $(EnableControlFlowGuard) /utf-8 %(AdditionalOptions) - /d2inlinelogfull:_PyEval_EvalFrameDefault %(AdditionalOptions) true From b7dfcdc916e7cfe0beb966bd015d20b278f31d1a Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 13 Apr 2022 22:03:07 +0000 Subject: [PATCH 14/23] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-04-13-22-03-04.gh-issue-89279.-jAVxZ.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-04-13-22-03-04.gh-issue-89279.-jAVxZ.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-04-13-22-03-04.gh-issue-89279.-jAVxZ.rst b/Misc/NEWS.d/next/Core and Builtins/2022-04-13-22-03-04.gh-issue-89279.-jAVxZ.rst new file mode 100644 index 00000000000000..66b1c884ec4508 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-04-13-22-03-04.gh-issue-89279.-jAVxZ.rst @@ -0,0 +1 @@ +Improve interpreter performance on Windows by inlining a few specific macros. From 9a5c57caee78c31ed99cc42e99bfe1d4b749498c Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 19 Apr 2022 12:29:45 -0700 Subject: [PATCH 15/23] Use static_assert() in _Py_atomic_load_relaxed_int32 Co-authored-by: Victor Stinner --- Python/ceval.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index f8a1afc98a0963..e93bd8a2d123d4 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -64,7 +64,9 @@ // GH-89279: Similar to above, force inlining by using a macro. #if defined(_MSC_VER) && SIZEOF_INT == 4 -#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) +#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) \ + (static_assert(sizeof((ATOMIC_VAL)->_value) == 4, "expect 32-bit int"), \ + *((volatile int*)&((ATOMIC_VAL)->_value))) #else #define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) _Py_atomic_load_relaxed(ATOMIC_VAL) #endif From a8cba6e659a88778a9ed4e7d775d7ecb93c680a0 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 19 Apr 2022 12:25:34 -0700 Subject: [PATCH 16/23] Revert "MSC is better off with the default Py_UNREACHABLE()" The evidence that `__assume(0)` is harmful is unclear and at best seems to only apply to the x86 platform (i.e., 32-bit), which we care about less and less. So revert this change. This reverts commit 5617fcdf08dda8ce87a81f8d8b190715091b4564. --- Include/pymacro.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Include/pymacro.h b/Include/pymacro.h index 4e2188379f02af..2728496976de7e 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -122,6 +122,8 @@ # define Py_UNREACHABLE() __builtin_unreachable() #elif defined(__clang__) || defined(__INTEL_COMPILER) # define Py_UNREACHABLE() __builtin_unreachable() +#elif defined(_MSC_VER) +# define Py_UNREACHABLE() __assume(0) #else # define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") From c43ec5632c09e9e6af5058601e004f1b8e1b09f1 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 19 Apr 2022 12:33:54 -0700 Subject: [PATCH 17/23] Add comment referring to GH-89279 for is_method() --- Python/ceval.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/ceval.c b/Python/ceval.c index e93bd8a2d123d4..edab33f561b4b1 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1599,6 +1599,7 @@ typedef struct { PyObject *kwnames; } CallShape; +// GH-89279: Must be a macro to be sure it's inlined by MSVC. #define is_method(stack_pointer, args) (PEEK((args)+2) != NULL) #define KWNAMES_LEN() \ From 9a15194f446768d7c7f7cf490cd4f61883a8da97 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 19 Apr 2022 13:14:40 -0700 Subject: [PATCH 18/23] static_assert() is only a C++ feature In GCC there's _Static_assert(), but MSVC only has _STATIC_ASSERT and that requires crtdbg.h. I don't think it's worth the hassle. Revert "Use static_assert() in _Py_atomic_load_relaxed_int32" This reverts commit 9a5c57caee78c31ed99cc42e99bfe1d4b749498c. --- Python/ceval.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index edab33f561b4b1..33341fbf0132ee 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -64,9 +64,7 @@ // GH-89279: Similar to above, force inlining by using a macro. #if defined(_MSC_VER) && SIZEOF_INT == 4 -#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) \ - (static_assert(sizeof((ATOMIC_VAL)->_value) == 4, "expect 32-bit int"), \ - *((volatile int*)&((ATOMIC_VAL)->_value))) +#define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) #else #define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) _Py_atomic_load_relaxed(ATOMIC_VAL) #endif From 7b342e4c7527c1b5d998fb11ee7e2dd866235220 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 20 Apr 2022 15:58:48 -0700 Subject: [PATCH 19/23] Also redefine _Py_DECREF_SPECIALIZED as a macro --- Python/ceval.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index e5759c284b54a2..096689d0585939 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -60,6 +60,8 @@ #undef Py_XDECREF #define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) +#undef _Py_DECREF_SPECIALIZED +#define _Py_DECREF_SPECIALIZED(arg, dealloc) do { PyObject *op = _PyObject_CAST(arg); if (--op->ob_refcnt == 0) { (dealloc)(op); } } while (0) #endif // GH-89279: Similar to above, force inlining by using a macro. From 3b21c5224c86e92fc484bb78bec9d5d5a3c3d69b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 21 Apr 2022 10:37:05 -0700 Subject: [PATCH 20/23] Reformat macros for readability Co-authored-by: Victor Stinner --- Python/ceval.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 096689d0585939..2cf9c7272177e6 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -52,16 +52,36 @@ // preprocessor. #undef Py_DECREF -#define Py_DECREF(arg) do { PyObject *op = _PyObject_CAST(arg); if (--op->ob_refcnt == 0) { destructor d = Py_TYPE(op)->tp_dealloc; (*d)(op); } } while (0) - -#undef Py_IS_TYPE -#define Py_IS_TYPE(ob, type) (_PyObject_CAST(ob)->ob_type == (type)) +#define Py_DECREF(arg) \ + do { \ + PyObject *op = _PyObject_CAST(arg); \ + if (--op->ob_refcnt == 0) { \ + destructor dealloc = Py_TYPE(op)->tp_dealloc; \ + (*dealloc)(op); \ + } \ + } while (0) #undef Py_XDECREF -#define Py_XDECREF(arg) do { PyObject *op1 = _PyObject_CAST(arg); if (op1 != NULL) { Py_DECREF(op1); } } while (0) +#define Py_XDECREF(arg) \ + do { \ + PyObject *xop = _PyObject_CAST(arg); \ + if (xop != NULL) { \ + Py_DECREF(xop); \ + } \ + } while (0) + +#undef Py_IS_TYPE +#define Py_IS_TYPE(ob, type) \ + (_PyObject_CAST_CONST(ob)->ob_type == (type)) #undef _Py_DECREF_SPECIALIZED -#define _Py_DECREF_SPECIALIZED(arg, dealloc) do { PyObject *op = _PyObject_CAST(arg); if (--op->ob_refcnt == 0) { (dealloc)(op); } } while (0) +#define _Py_DECREF_SPECIALIZED(arg, dealloc) \ + do { \ + PyObject *op = _PyObject_CAST(arg); \ + if (--op->ob_refcnt == 0) { \ + (dealloc)(op); \ + } \ + } while (0) #endif // GH-89279: Similar to above, force inlining by using a macro. From 41cb0674cba65ee6793e2db065d254b803aceac8 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 21 Apr 2022 13:19:26 -0700 Subject: [PATCH 21/23] Try to silence warnings Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com> --- Python/ceval.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 2cf9c7272177e6..f837f2cdd15e9c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -79,7 +79,8 @@ do { \ PyObject *op = _PyObject_CAST(arg); \ if (--op->ob_refcnt == 0) { \ - (dealloc)(op); \ + destructor d = (destructor)(dealloc); + d(op); \ } \ } while (0) #endif From 87aaf81ca75fc7a8d0f3dbe8b1537c766c178d3e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 21 Apr 2022 22:26:44 +0200 Subject: [PATCH 22/23] Update Python/ceval.c --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index f837f2cdd15e9c..608011d0717fd9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -72,7 +72,7 @@ #undef Py_IS_TYPE #define Py_IS_TYPE(ob, type) \ - (_PyObject_CAST_CONST(ob)->ob_type == (type)) + (_PyObject_CAST(ob)->ob_type == (type)) #undef _Py_DECREF_SPECIALIZED #define _Py_DECREF_SPECIALIZED(arg, dealloc) \ From 3508f455b1332c2f6752c76cd20902bf6a35ce4e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 21 Apr 2022 22:26:51 +0200 Subject: [PATCH 23/23] Update Python/ceval.c --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 608011d0717fd9..919e0ac9575586 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -79,7 +79,7 @@ do { \ PyObject *op = _PyObject_CAST(arg); \ if (--op->ob_refcnt == 0) { \ - destructor d = (destructor)(dealloc); + destructor d = (destructor)(dealloc); \ d(op); \ } \ } while (0)