Message352688
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. |
|
Date |
User |
Action |
Args |
2019-09-18 04:39:27 | Greg Price | set | recipients:
+ Greg Price, rhettinger, mark.dickinson, vstinner, malin, sir-sigurd, aeros |
2019-09-18 04:39:27 | Greg Price | set | messageid: <1568781567.74.0.675607050568.issue37812@roundup.psfhosted.org> |
2019-09-18 04:39:27 | Greg Price | link | issue37812 messages |
2019-09-18 04:39:27 | Greg Price | create | |
|