[PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Romain Naour]

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(+)

diff --git a/ChangeLog b/ChangeLog
index f3b5e8c..de87072 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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
diff --git a/math/math.h b/math/math.h
index faa24817..5ad8156 100644
--- a/math/math.h
+++ b/math/math.h
@@ -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
-- 
2.9.5

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Joseph Myers]

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).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Gabriel F. T. Gomes]

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).

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Joseph Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Gabriel F. T. Gomes]

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).

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Gabriel F. T. Gomes]

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

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Joseph Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Romain Naour]

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

Re: [PATCH v2] Let signbit use the builtin in C++ mode with gcc < 6.x (bug 22146)

[Romain Naour]

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