Message ID | 20171015193652.28657-1-romain.naour@gmail.com |
---|---|
State | New |
Headers | show |
Series | Provide a C++ version of signbit that does not use __MATH_TG (bug 22296) | expand |
On Sun, 15 Oct 2017, Romain Naour wrote: >--- a/math/math.h >+++ b/math/math.h >@@ -448,6 +448,21 @@ enum > /* Return nonzero value if sign of X is negative. */ > # if __GNUC_PREREQ (6,0) > # define signbit(x) __builtin_signbit (x) >+# elif defined __cplusplus I believe that the comment [1] about fpclassify applies to signbit, too, because it is also the job of libstdc++ to provide it in C++11. [1] https://sourceware.org/ml/libc-alpha/2017-09/msg00787.html Adding a check for the standard in use (__cpluplus < 201103L) would probably be safer and it would solve the problem with mesa, as described in bug 22296. However, it would not help with the configure check in libstdc++ ("checking for ISO C99 support in <math.h> for C++11"). Moreover, I'm not sure if defining signbit as a function in math.h is correct, even if it's only for C++11. So, I suggest we wait for comments from other, more experienced developers. :) >+inline int signbit (long double __val) { return __signbitl (__val); } If defining signbit as a function in math.h is actually OK, then you need to check for __NO_LONG_DOUBLE_MATH before calling __signbitl, since it is only defined when long double is not the same as double. Otherwise, call __signbit.
On Mon, 16 Oct 2017, Gabriel F. T. Gomes wrote: > On Sun, 15 Oct 2017, Romain Naour wrote: > > >--- a/math/math.h > >+++ b/math/math.h > >@@ -448,6 +448,21 @@ enum > > /* Return nonzero value if sign of X is negative. */ > > # if __GNUC_PREREQ (6,0) > > # define signbit(x) __builtin_signbit (x) > >+# elif defined __cplusplus > > I believe that the comment [1] about fpclassify applies to signbit, too, > because it is also the job of libstdc++ to provide it in C++11. Yes, this case is like isinf and fpclassify: it's the job of libstdc++ to undefine these macros, and redefine as functions if necessary. None of the C99 type-generic macros should be defined by glibc as functions for C++, because that's libstdc++'s job; the glibc definitions should only ever be used for C++ when running the libstdc++ configure test. The TS 18661 macros like issignaling are different because they aren't in standard C++ and libstdc++ doesn't provide its own versions of them. My suggestion for signbit would be just to call __builtin_signbitl for the case of (C++, !__GNUC_PREREQ (6,0)), which may involve unnecessary conversions to long double when called at runtime (which shouldn't happen anyway) but should be sufficient for the configure test.
Hi Gabriel, Joseph, Le 16/10/2017 à 19:18, Joseph Myers a écrit : > On Mon, 16 Oct 2017, Gabriel F. T. Gomes wrote: > >> On Sun, 15 Oct 2017, Romain Naour wrote: >> >>> --- a/math/math.h >>> +++ b/math/math.h >>> @@ -448,6 +448,21 @@ enum >>> /* Return nonzero value if sign of X is negative. */ >>> # if __GNUC_PREREQ (6,0) >>> # define signbit(x) __builtin_signbit (x) >>> +# elif defined __cplusplus >> >> I believe that the comment [1] about fpclassify applies to signbit, too, >> because it is also the job of libstdc++ to provide it in C++11. > > Yes, this case is like isinf and fpclassify: it's the job of libstdc++ to > undefine these macros, and redefine as functions if necessary. None of > the C99 type-generic macros should be defined by glibc as functions for > C++, because that's libstdc++'s job; the glibc definitions should only > ever be used for C++ when running the libstdc++ configure test. The TS > 18661 macros like issignaling are different because they aren't in > standard C++ and libstdc++ doesn't provide its own versions of them. > > My suggestion for signbit would be just to call __builtin_signbitl for the > case of (C++, !__GNUC_PREREQ (6,0)), which may involve unnecessary > conversions to long double when called at runtime (which shouldn't happen > anyway) but should be sufficient for the configure test. Thanks for the review. I'm not very familiar on the glibc code and how it interact with gcc build system... But here is the updated patch following your advice: https://sourceware.org/ml/libc-alpha/2017-10/msg00725.html Best regards, Romain
diff --git a/ChangeLog b/ChangeLog index 17fbe55..2fe5719 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2017-10-15 Romain Naour <romain.naour@gmail.com> (tiny change) + + [BZ #22296] + * math/math.h [defined __cplusplus] (signbit): Provide a C++ + definition for signbit that does not rely on __MATH_TG, + since __MATH_TG uses __builtin_types_compatible_p, which is only + available in C mode. + 2017-10-15 H.J. Lu <hongjiu.lu@intel.com> [BZ #22052] diff --git a/math/math.h b/math/math.h index faa24817..2c94cdf 100644 --- a/math/math.h +++ b/math/math.h @@ -448,6 +448,21 @@ enum /* Return nonzero value if sign of X is negative. */ # if __GNUC_PREREQ (6,0) # define signbit(x) __builtin_signbit (x) +# elif defined __cplusplus + /* In C++ mode, __MATH_TG cannot be used, because it relies on + __builtin_types_compatible_p, which is a C-only builtin. On the + other hand, overloading provides the means to distinguish between + the floating-point types. The overloading resolution will match + the correct parameter (regardless of type qualifiers (i.e.: const + and volatile). */ +extern "C++" { +inline int signbit (float __val) { return __signbitf (__val); } +inline int signbit (double __val) { return __signbit (__val); } +inline int signbit (long double __val) { return __signbitl (__val); } +# if __HAVE_DISTINCT_FLOAT128 +inline int signbit (_Float128 __val) { return __signbitf128 (__val); } +# endif +} /* extern C++ */ # elif __GNUC_PREREQ (4,0) # define signbit(x) __MATH_TG ((x), __builtin_signbit, (x)) # else