[v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)
Commit Message
When using gcc < 6.x, signbit does not use the type-generic
__builtin_signbit builtin, instead it uses __MATH_TG.
However, when library support for float128 is available, __MATH_TG uses
__builtin_types_compatible_p, which is not available in C++ mode.
On the other hand, libstdc++ undefines (in cmath) many macros from
math.h, including signbit, so that it can provide its own functions.
However, during its configure tests, libstdc++ just tests for the
availability of the macros (it does not undefine them, nor does it
provide its own functions).
Finally, when libstdc++ is configured with optimization for size
enabled, its configure tests include math.h and get the definition of
signbit that uses __MATH_TG (and __builtin_types_compatible_p).
Since libstdc++ does not undefine the macros during its configure
tests, they fail.
This patch lets signbit use the builtin in C++ mode when gcc < 6.x is
used. This allows the configure test in libstdc++ to work.
Tested for x86_64.
[BZ #22296]
math/math.h: Let signbit use the builtin in C++ mode with gcc
< 6.x
Cc: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com>
Cc: Joseph Myers <joseph@codesourcery.com>
---
v2
Rework the patch following the review:
https://sourceware.org/ml/libc-alpha/2017-09/msg00787.html
---
ChangeLog | 5 +++++
math/math.h | 9 +++++++++
2 files changed, 14 insertions(+)
Comments
This patch version is OK (with ChangeLog syntax fixed - missing "* " at
the start of the ChangeLog entry and missing blank line before the next
entry).
On Mon, 16 Oct 2017, Joseph Myers wrote:
>This patch version is OK (with ChangeLog syntax fixed - missing "* " at
>the start of the ChangeLog entry and missing blank line before the next
>entry).
Just to confirm, this change does not require copyright assignment from
Romain, right? (I don't know if Romain has it).
On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
> On Mon, 16 Oct 2017, Joseph Myers wrote:
>
> >This patch version is OK (with ChangeLog syntax fixed - missing "* " at
> >the start of the ChangeLog entry and missing blank line before the next
> >entry).
>
> Just to confirm, this change does not require copyright assignment from
> Romain, right? (I don't know if Romain has it).
This change is small enough that it does not require copyright assignment.
On Tue, 17 Oct 2017, Joseph Myers wrote:
>On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>
>> On Mon, 16 Oct 2017, Joseph Myers wrote:
>>
>> >This patch version is OK (with ChangeLog syntax fixed - missing "* " at
>> >the start of the ChangeLog entry and missing blank line before the next
>> >entry).
>>
>> Just to confirm, this change does not require copyright assignment from
>> Romain, right? (I don't know if Romain has it).
>
>This change is small enough that it does not require copyright assignment.
OK. I'm testing on powerpc64le and then I'll check this patch in on
Romain's behalf (with Romain as git author, as usual, and with the fixes to
the ChangeLog entry).
On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>OK. I'm testing on powerpc64le and then I'll check this patch in on
>Romain's behalf (with Romain as git author, as usual, and with the fixes to
>the ChangeLog entry).
Now committed with the changes mentioned above, and with the bug number
fixed in the commit headline.
I can backport this to the stable branch, but after some days, as
suggested in https://sourceware.org/ml/libc-alpha/2017-10/msg00166.html
On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
> On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>
> >OK. I'm testing on powerpc64le and then I'll check this patch in on
> >Romain's behalf (with Romain as git author, as usual, and with the fixes to
> >the ChangeLog entry).
>
> Now committed with the changes mentioned above, and with the bug number
> fixed in the commit headline.
Please make sure to mark the bug FIXED with target milestone set.
Hi,
Le 17/10/2017 à 18:39, Joseph Myers a écrit :
> On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>
>> On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>>
>>> OK. I'm testing on powerpc64le and then I'll check this patch in on
>>> Romain's behalf (with Romain as git author, as usual, and with the fixes to
>>> the ChangeLog entry).
>>
>> Now committed with the changes mentioned above, and with the bug number
>> fixed in the commit headline.
>
> Please make sure to mark the bug FIXED with target milestone set.
>
Thanks for the final test and the merge.
I've marked this bug as resolved and fixed but I can't change the Target Milestone.
Best regards,
Romain
Hi Gabriel,
Le 17/10/2017 à 16:21, Gabriel F. T. Gomes a écrit :
> On Tue, 17 Oct 2017, Gabriel F. T. Gomes wrote:
>
>> OK. I'm testing on powerpc64le and then I'll check this patch in on
>> Romain's behalf (with Romain as git author, as usual, and with the fixes to
>> the ChangeLog entry).
>
> Now committed with the changes mentioned above, and with the bug number
> fixed in the commit headline.
>
> I can backport this to the stable branch, but after some days, as
> suggested in https://sourceware.org/ml/libc-alpha/2017-10/msg00166.html
>
With Yann E. Morin, we are going to switch the glibc package in Buildroot to the
2.26 stable branch. Buildroot still support building gcc 5.x toolchain and we
needs this patch before the next Buildroot stable freeze at the end of this month.
Just don't forget to backport it.
Thanks again for the help!
Best regards,
Romain
@@ -1,3 +1,8 @@
+2017-10-16 Romain Naour <romain.naour@gmail.com> (tiny change)
+
+ [BZ #22296]
+ math/math.h: Let signbit use the builtin in C++ mode with gcc
+ < 6.x
2017-10-16 Florian Weimer <fweimer@redhat.com>
* version.h (VERSION): Switch to ".9000" as the development
@@ -448,6 +448,15 @@ 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.
+ The check for __cplusplus allows the use of the builtin instead of
+ __MATH_TG. This is provided for libstdc++, only to let its configure
+ test work. No further use of this definition of signbit is expected
+ in C++ mode, since libstdc++ provides its own version of signbit
+ in cmath (which undefines signbit). */
+# define signbit(x) __builtin_signbitl (x)
# elif __GNUC_PREREQ (4,0)
# define signbit(x) __MATH_TG ((x), __builtin_signbit, (x))
# else