This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Provide a C++ version of signbit that does not use __MATH_TG (bug 22296)
- From: Romain Naour <romain dot naour at gmail dot com>
- To: Joseph Myers <joseph at codesourcery dot com>, "Gabriel F. T. Gomes" <gftg at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 16 Oct 2017 23:28:02 +0200
- Subject: Re: [PATCH] Provide a C++ version of signbit that does not use __MATH_TG (bug 22296)
- Authentication-results: sourceware.org; auth=none
- References: <20171015193652.28657-1-romain.naour@gmail.com> <20171016103140.4382f672@keller> <alpine.DEB.2.20.1710161713470.30518@digraph.polyomino.org.uk>
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