Message ID | 2169197.iZASKD2KPV@minbar |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 04010385780E for <patchwork@sourceware.org>; Wed, 19 Jan 2022 08:09:42 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from lxmtout2.gsi.de (lxmtout2.gsi.de [140.181.3.112]) by sourceware.org (Postfix) with ESMTPS id 0FA4D3857827; Wed, 19 Jan 2022 08:07:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0FA4D3857827 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gsi.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gsi.de Received: from localhost (localhost [127.0.0.1]) by lxmtout2.gsi.de (Postfix) with ESMTP id 5ADE1203E7FC; Wed, 19 Jan 2022 09:07:56 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at lxmtout2.gsi.de Received: from lxmtout2.gsi.de ([127.0.0.1]) by localhost (lxmtout2.gsi.de [127.0.0.1]) (amavisd-new, port 10024) with LMTP id G43R0MDI07oA; Wed, 19 Jan 2022 09:07:56 +0100 (CET) Received: from srvex3.campus.gsi.de (unknown [10.10.4.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by lxmtout2.gsi.de (Postfix) with ESMTPS id 3D97B203E7E8; Wed, 19 Jan 2022 09:07:56 +0100 (CET) Received: from minbar.localnet (140.181.3.12) by srvex3.campus.gsi.de (10.10.4.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2375.18; Wed, 19 Jan 2022 09:07:55 +0100 From: Matthias Kretz <m.kretz@gsi.de> To: <gcc-patches@gcc.gnu.org>, <libstdc++@gcc.gnu.org> Subject: [PATCH] libstdc++: Fix for non-constexpr math_errhandling Date: Wed, 19 Jan 2022 09:07:55 +0100 Message-ID: <2169197.iZASKD2KPV@minbar> Organization: GSI Helmholtz Centre for Heavy Ion Research MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1881587.PYKUYFuaPT" Content-Transfer-Encoding: 7Bit X-Originating-IP: [140.181.3.12] X-ClientProxiedBy: SRVEX2.campus.gsi.de (10.10.4.15) To srvex3.campus.gsi.de (10.10.4.16) X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_PASS, TXREP, T_SPF_HELO_PERMERROR autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
libstdc++: Fix for non-constexpr math_errhandling
|
|
Commit Message
Matthias Kretz
Jan. 19, 2022, 8:07 a.m. UTC
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
──────────────────────────────────────────────────────────────────────────
Comments
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;