Message ID | 2169197.iZASKD2KPV@minbar |
---|---|
State | New |
Headers | show |
Series | libstdc++: Fix for non-constexpr math_errhandling | expand |
On Wed, 19 Jan 2022 at 08:10, Matthias Kretz wrote: > Follow-up to my last patch. This one is a more thorough fix. Tested on > x86_64- > linux. OK for trunk? > > ---- 8< ---- > > Use SFINAE magic to support: "It is unspecified whether math_errhandling > is a macro or an identifier with external linkage." [C Standard] > The patch is OK for trunk, but I don't understand what the C standard means here. "the macro math_errhandling expands to [...]. It is unspecified whether math_errhandling is a macro or an identifier with external linkage." So is it a macro or not?
On Wednesday, 19 January 2022 13:07:26 CET Jonathan Wakely wrote: > On Wed, 19 Jan 2022 at 08:10, Matthias Kretz wrote: > > Follow-up to my last patch. This one is a more thorough fix. Tested on > > x86_64- > > linux. OK for trunk? > > > > ---- 8< ---- > > > > Use SFINAE magic to support: "It is unspecified whether math_errhandling > > is a macro or an identifier with external linkage." [C Standard] > > The patch is OK for trunk, but I don't understand what the C standard means > here. > > "the macro math_errhandling expands to [...]. It is unspecified whether > math_errhandling is a macro or an identifier with external linkage." > > So is it a macro or not? I agree the quote I used is unclear. The complete paragraph: The macros MATH_ERRNO MATH_ERREXCEPT expand to the integer constants 1 and 2, respectively; the macro math_errhandling expands to an expression that has type int and the value MATH_ERRNO, MATH_ERREXCEPT, or the bitwise OR of both. The value of math_errhandling is constant for the duration of the program. It is unspecified whether math_errhandling is a macro or an identifier with external linkage. If a macro definition is suppressed or a program defines an identifier with the name math_errhandling, the behavior is undefined. If the expression math_errhandling & MATH_ERREXCEPT can be nonzero, the implementation shall define the macros FE_DIVBYZERO, FE_INVALID, and FE_OVERFLOW in <fenv.h>.
On Wed, 19 Jan 2022, 12:45 Matthias Kretz, <m.kretz@gsi.de> wrote: > On Wednesday, 19 January 2022 13:07:26 CET Jonathan Wakely wrote: > > On Wed, 19 Jan 2022 at 08:10, Matthias Kretz wrote: > > > Follow-up to my last patch. This one is a more thorough fix. Tested on > > > x86_64- > > > linux. OK for trunk? > > > > > > ---- 8< ---- > > > > > > Use SFINAE magic to support: "It is unspecified whether > math_errhandling > > > is a macro or an identifier with external linkage." [C Standard] > > > > The patch is OK for trunk, but I don't understand what the C standard > means > > here. > > > > "the macro math_errhandling expands to [...]. It is unspecified whether > > math_errhandling is a macro or an identifier with external linkage." > > > > So is it a macro or not? > > I agree the quote I used is unclear. The complete paragraph: > > The macros > > MATH_ERRNO > MATH_ERREXCEPT > > expand to the integer constants 1 and 2, respectively; the macro > > math_errhandling > > expands to an expression that has type int and the value MATH_ERRNO, > MATH_ERREXCEPT, or the bitwise OR of both. The value of math_errhandling > is > constant for the duration of the program. It is unspecified whether > math_errhandling is a macro or an identifier with external linkage. If a > macro > definition is suppressed or a program defines an identifier with the name > math_errhandling, the behavior is undefined. If the expression > math_errhandling & MATH_ERREXCEPT can be nonzero, the implementation shall > define the macros FE_DIVBYZERO, FE_INVALID, and FE_OVERFLOW in > <fenv.h>. > But that still says "the macro math_errhandling" and then says it might not be a macro. I'll ask some WG14 people for clarity, but it doesn't affect your patch.
On Wednesday, 19 January 2022 16:21:15 CET Jonathan Wakely wrote: > On Wed, 19 Jan 2022, 12:45 Matthias Kretz, <m.kretz@gsi.de> wrote: > > On Wednesday, 19 January 2022 13:07:26 CET Jonathan Wakely wrote: > > > So is it a macro or not? > > > > I agree the quote I used is unclear. The complete paragraph: > > > > The macros > > > > MATH_ERRNO > > MATH_ERREXCEPT > > > > expand to the integer constants 1 and 2, respectively; the macro > > > > math_errhandling > > > > expands to an expression that has type int and the value MATH_ERRNO, > > MATH_ERREXCEPT, or the bitwise OR of both. The value of math_errhandling > > is > > constant for the duration of the program. It is unspecified whether > > math_errhandling is a macro or an identifier with external linkage. If a > > macro > > definition is suppressed or a program defines an identifier with the name > > math_errhandling, the behavior is undefined. If the expression > > math_errhandling & MATH_ERREXCEPT can be nonzero, the implementation shall > > define the macros FE_DIVBYZERO, FE_INVALID, and FE_OVERFLOW in > > <fenv.h>. > > But that still says "the macro math_errhandling" and then says it might not > be a macro. There's also [cmath.syn] https://eel.is/c++draft/cmath.syn which says: #define math_errhandling see below So, FWIW, libstdc++ is required to define math_errhandling as a macro in <cmath>. Thus, the original error (that math_errhandling wasn't defined even after <cmath> was included) really needs a fix in <cmath>. :-P
On Wed, 19 Jan 2022 at 15:46, Matthias Kretz <m.kretz@gsi.de> wrote: > > On Wednesday, 19 January 2022 16:21:15 CET Jonathan Wakely wrote: > > On Wed, 19 Jan 2022, 12:45 Matthias Kretz, <m.kretz@gsi.de> wrote: > > > On Wednesday, 19 January 2022 13:07:26 CET Jonathan Wakely wrote: > > > > So is it a macro or not? > > > > > > I agree the quote I used is unclear. The complete paragraph: > > > > > > The macros > > > > > > MATH_ERRNO > > > MATH_ERREXCEPT > > > > > > expand to the integer constants 1 and 2, respectively; the macro > > > > > > math_errhandling > > > > > > expands to an expression that has type int and the value MATH_ERRNO, > > > MATH_ERREXCEPT, or the bitwise OR of both. The value of math_errhandling > > > is > > > constant for the duration of the program. It is unspecified whether > > > math_errhandling is a macro or an identifier with external linkage. If a > > > macro > > > definition is suppressed or a program defines an identifier with the name > > > math_errhandling, the behavior is undefined. If the expression > > > math_errhandling & MATH_ERREXCEPT can be nonzero, the implementation shall > > > define the macros FE_DIVBYZERO, FE_INVALID, and FE_OVERFLOW in > > > <fenv.h>. > > > > But that still says "the macro math_errhandling" and then says it might not > > be a macro. > > There's also [cmath.syn] https://eel.is/c++draft/cmath.syn which says: > > #define math_errhandling see below > > So, FWIW, libstdc++ is required to define math_errhandling as a macro in > <cmath>. Thus, the original error (that math_errhandling wasn't defined even > after <cmath> was included) really needs a fix in <cmath>. :-P No, because we get it from libc: #include_next <math.h>
On Wed, 19 Jan 2022 at 16:45, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > On Wed, 19 Jan 2022 at 15:46, Matthias Kretz <m.kretz@gsi.de> wrote: > > > > On Wednesday, 19 January 2022 16:21:15 CET Jonathan Wakely wrote: > > > On Wed, 19 Jan 2022, 12:45 Matthias Kretz, <m.kretz@gsi.de> wrote: > > > > On Wednesday, 19 January 2022 13:07:26 CET Jonathan Wakely wrote: > > > > > So is it a macro or not? > > > > > > > > I agree the quote I used is unclear. The complete paragraph: > > > > > > > > The macros > > > > > > > > MATH_ERRNO > > > > MATH_ERREXCEPT > > > > > > > > expand to the integer constants 1 and 2, respectively; the macro > > > > > > > > math_errhandling > > > > > > > > expands to an expression that has type int and the value MATH_ERRNO, > > > > MATH_ERREXCEPT, or the bitwise OR of both. The value of math_errhandling > > > > is > > > > constant for the duration of the program. It is unspecified whether > > > > math_errhandling is a macro or an identifier with external linkage. If a > > > > macro > > > > definition is suppressed or a program defines an identifier with the name > > > > math_errhandling, the behavior is undefined. If the expression > > > > math_errhandling & MATH_ERREXCEPT can be nonzero, the implementation shall > > > > define the macros FE_DIVBYZERO, FE_INVALID, and FE_OVERFLOW in > > > > <fenv.h>. > > > > > > But that still says "the macro math_errhandling" and then says it might not > > > be a macro. > > > > There's also [cmath.syn] https://eel.is/c++draft/cmath.syn which says: > > > > #define math_errhandling see below > > > > So, FWIW, libstdc++ is required to define math_errhandling as a macro in > > <cmath>. Thus, the original error (that math_errhandling wasn't defined even > > after <cmath> was included) really needs a fix in <cmath>. :-P > > No, because we get it from libc: > > #include_next <math.h> So if you aren't seeing it after <cmath> is included, your libc is broken.
diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h index c991e3f223e..82e9841195e 100644 --- a/libstdc++-v3/include/experimental/bits/simd.h +++ b/libstdc++-v3/include/experimental/bits/simd.h @@ -283,20 +283,34 @@ constexpr inline bool __have_power_vmx = __have_power_vsx; namespace __detail { - constexpr bool __handle_fpexcept = #ifdef math_errhandling - math_errhandling & MATH_ERREXCEPT; -#elif defined __FAST_MATH__ - false; + // Determines _S_handle_fpexcept from math_errhandling if it is defined and expands to a constant + // expression. math_errhandling may expand to an extern symbol, in which case a constexpr value + // must be guessed. + template <int = math_errhandling> + constexpr bool __handle_fpexcept_impl(int) + { return math_errhandling & MATH_ERREXCEPT; } +#endif + + // Fallback if math_errhandling doesn't work: with fast-math assume floating-point exceptions are + // ignored, otherwise implement correct exception behavior. + constexpr bool __handle_fpexcept_impl(float) + { +#if defined __FAST_MATH__ + return false; #else - true; + return true; #endif + } + + /// True if math functions must raise floating-point exceptions as specified by C17. + static constexpr bool _S_handle_fpexcept = __handle_fpexcept_impl(0); constexpr std::uint_least64_t __floating_point_flags() { std::uint_least64_t __flags = 0; - if constexpr (__handle_fpexcept) + if constexpr (_S_handle_fpexcept) __flags |= 1; #ifdef __FAST_MATH__ __flags |= 1 << 1;
Follow-up to my last patch. This one is a more thorough fix. Tested on x86_64- linux. OK for trunk? ---- 8< ---- Use SFINAE magic to support: "It is unspecified whether math_errhandling is a macro or an identifier with external linkage." [C Standard] Signed-off-by: Matthias Kretz <m.kretz@gsi.de> libstdc++-v3/ChangeLog: * include/experimental/bits/simd.h (__floating_point_flags): Do not rely on math_errhandling to expand to a constant expression. --- libstdc++-v3/include/experimental/bits/simd.h | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) -- ────────────────────────────────────────────────────────────────────────── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de stdₓ::simd ──────────────────────────────────────────────────────────────────────────