This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author Greg Price
Recipients Greg Price, aeros, malin, mark.dickinson, rhettinger, sir-sigurd, vstinner
Date 2019-09-18.04:39:27
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1568781567.74.0.675607050568.issue37812@roundup.psfhosted.org>
In-reply-to
Content
Thanks Victor for linking that issue back here.

> A first change converted a macro to a static inline function. The second change converted the static inline fnuction to a macro

Not quite. The first change converted a macro `CHECK_SMALL_INT` to an equivalent sequence, including a `return` that had been hidden inside the macro:

    if (is_small_int(ival)) {
        return get_small_int((sdigit)ival);
    }

The second change converted `is_small_int` to a macro `IS_SMALL_INT`.

The second change also changed an `assert(0)` to say `Py_UNREACHABLE()`. I don't know why it did that -- it seems quite orthogonal to the rest of the change.


> Morever, if using a static inline function is causing issues,

The change that caused the build failure you're seeing (GH-15710) was intended for bpo-38015 .  Details there; briefly, common compilers on x86_32 would emit some unsightly extra instructions with `is_small_int`, and converting it to the macro `IS_SMALL_INT` eliminated those extra instructions.

It's not clear to me if anyone benchmarked to see if the conversion to a macro had any measurable performance benefit.  There's some measurement here:
https://bugs.python.org/issue38015#msg351315
which finds no benefit.  I'm not sure if that was measuring the change in GH-15710, or if it was an attempt to replicate the findings in msg351255 (which it quotes) about a more invasive change.

So I think the short of it is that the static inline function was not causing any issue, and an easy fix for the build failure is to revert GH-15710.

Alternatively, another easy fix would be to replace just the `Py_UNREACHABLE()` introduced by GH-15710 with the original `assert(0)`.

Either way, I don't think there's any reason here to revert GH-15216.
History
Date User Action Args
2019-09-18 04:39:27Greg Pricesetrecipients: + Greg Price, rhettinger, mark.dickinson, vstinner, malin, sir-sigurd, aeros
2019-09-18 04:39:27Greg Pricesetmessageid: <1568781567.74.0.675607050568.issue37812@roundup.psfhosted.org>
2019-09-18 04:39:27Greg Pricelinkissue37812 messages
2019-09-18 04:39:27Greg Pricecreate