[PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Peter Bergner]

Usage of getauxval() within the float128 libgcc IFUNC resolver functions is
causing problems:

    https://sourceware.org/bugzilla/show_bug.cgi?id=21707

Alan describes why we can't have relocations in IFUNC resolver functions here:

    https://gcc.gnu.org/PR81193

With the addition of __builtin_cpu_supports (), we no longer need to call
getauxval() to query the HWCAP/HWCAP2 masks, so let's use that instead.

I have verified with some small test cases, that we do call the correct
__{add,sub,...}k3_hw() functions instead of the *_sw versions.  I did that
by running the test cases in GDB and manually setting the IEEE128 bit in
the HWCAP2 mask stored in the TCB before the resolvers were run.

This bootstrapps and regtests with no regressions, ok for trunk?

I will note that this patch causes issues in some tests in the GLIBC testsiute,
which Tulio is working on fixing (it's a GLIBC issue, not a GCC issue), so if
this patch is "ok", I plan on holding off on committing this, until the GLIBC
fix is committed.

Peter

	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
	(have_ieee_hw_p): Delete function.
	(SW_OR_HW) Use __builtin_cpu_supports().

Index: libgcc/config/rs6000/float128-ifunc.c
===================================================================
--- libgcc/config/rs6000/float128-ifunc.c	(revision 249850)
+++ libgcc/config/rs6000/float128-ifunc.c	(working copy)
@@ -45,47 +45,7 @@
 #error "This module must not be compiled with IEEE 128-bit hardware support"
 #endif
 
-#include <sys/auxv.h>
-
-/* Use the namespace clean version of getauxval.  However, not all versions of
-   sys/auxv.h declare it, so declare it here.  This code is intended to be
-   temporary until a suitable version of __builtin_cpu_supports is added that
-   allows us to tell quickly if the machine supports IEEE 128-bit hardware.  */
-extern unsigned long __getauxval (unsigned long);
-
-static int
-have_ieee_hw_p (void)
-{
-  static int ieee_hw_p = -1;
-
-  if (ieee_hw_p < 0)
-    {
-      char *p = (char *) __getauxval (AT_PLATFORM);
-
-      ieee_hw_p = 0;
-
-      /* Don't use atoi/strtol/strncmp/etc.  These may require the normal
-	 environment to be setup to set errno to 0, and the ifunc resolvers run
-	 before the whole glibc environment is initialized.  */
-      if (p && p[0] == 'p' && p[1] == 'o' && p[2] == 'w' && p[3] == 'e'
-	  && p[4] == 'r')
-	{
-	  long n = 0;
-	  char ch;
-
-	  p += 5;
-	  while ((ch = *p++) >= '0' && (ch <= '9'))
-	    n = (n * 10) + (ch - '0');
-
-	  if (n >= 9)
-	    ieee_hw_p = 1;
-	}
-    }
-
-  return ieee_hw_p;
-}
-
-#define SW_OR_HW(SW, HW) (have_ieee_hw_p () ? HW : SW)
+#define SW_OR_HW(SW, HW) (__builtin_cpu_supports ("ieee128") ? HW : SW)
 
 /* Resolvers.  */
 

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Florian Weimer]

On 07/06/2017 11:21 PM, Peter Bergner wrote:
> I will note that this patch causes issues in some tests in the GLIBC testsiute,
> which Tulio is working on fixing (it's a GLIBC issue, not a GCC issue), so if
> this patch is "ok", I plan on holding off on committing this, until the GLIBC
> fix is committed.

This issue currently blocks Fedora rawhide development.  I would prefer
if we could move this forward despite temporary glibc test suite failures.

Thanks,
Florian

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Segher Boessenkool]

On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
> 	(have_ieee_hw_p): Delete function.
> 	(SW_OR_HW) Use __builtin_cpu_supports().

Okay for trunk.  Thanks!


Segher

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Peter Bergner]

On 7/7/17 10:18 AM, Segher Boessenkool wrote:
> On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
>> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
>> 	(have_ieee_hw_p): Delete function.
>> 	(SW_OR_HW) Use __builtin_cpu_supports().
> 
> Okay for trunk.  Thanks!

Given Florian wants this in now to fix a Fedora blocker, I have
committed this to trunk.  We need the fix on GCC 6 and GCC 5 as
well, so ok there assuming bootstrapping / regtesting are clean?

Peter

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Peter Bergner]

On 7/7/17 4:13 PM, Peter Bergner wrote:
> On 7/7/17 10:18 AM, Segher Boessenkool wrote:
>> On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
>>> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
>>> 	(have_ieee_hw_p): Delete function.
>>> 	(SW_OR_HW) Use __builtin_cpu_supports().
>>
>> Okay for trunk.  Thanks!
> 
> Given Florian wants this in now to fix a Fedora blocker, I have
> committed this to trunk.  We need the fix on GCC 6 and GCC 5 as
> well, so ok there assuming bootstrapping / regtesting are clean?

FYI, the bootstrap and regtesting were clean on both the GCC 7 and
GCC 6 release branches.  Ok for those branches?

Peter

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Segher Boessenkool]

On Fri, Jul 07, 2017 at 07:14:25PM -0500, Peter Bergner wrote:
> On 7/7/17 4:13 PM, Peter Bergner wrote:
> > On 7/7/17 10:18 AM, Segher Boessenkool wrote:
> >> On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
> >>> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
> >>> 	(have_ieee_hw_p): Delete function.
> >>> 	(SW_OR_HW) Use __builtin_cpu_supports().
> >>
> >> Okay for trunk.  Thanks!
> > 
> > Given Florian wants this in now to fix a Fedora blocker, I have
> > committed this to trunk.  We need the fix on GCC 6 and GCC 5 as
> > well, so ok there assuming bootstrapping / regtesting are clean?
> 
> FYI, the bootstrap and regtesting were clean on both the GCC 7 and
> GCC 6 release branches.  Ok for those branches?

Okay for 7 and 6.  If it is a blocker (or you have another reason you
cannot wait to see if regressions pop up), please make sure you test it
on many different systems.


Segher

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Peter Bergner]

On 7/10/17 9:48 AM, Segher Boessenkool wrote:
> On Fri, Jul 07, 2017 at 07:14:25PM -0500, Peter Bergner wrote:
>> On 7/7/17 4:13 PM, Peter Bergner wrote:
>>> On 7/7/17 10:18 AM, Segher Boessenkool wrote:
>>>> On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
>>>>> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
>>>>> 	(have_ieee_hw_p): Delete function.
>>>>> 	(SW_OR_HW) Use __builtin_cpu_supports().
>>>>
>>>> Okay for trunk.  Thanks!
>>>
>>> Given Florian wants this in now to fix a Fedora blocker, I have
>>> committed this to trunk.  We need the fix on GCC 6 and GCC 5 as
>>> well, so ok there assuming bootstrapping / regtesting are clean?
>>
>> FYI, the bootstrap and regtesting were clean on both the GCC 7 and
>> GCC 6 release branches.  Ok for those branches?
> 
> Okay for 7 and 6.  If it is a blocker (or you have another reason you
> cannot wait to see if regressions pop up), please make sure you test it
> on many different systems.

The backports bootstrapped and regtested with no regressions on
powerpc64le-linux and powerpc64-linux, while running the testsuite
in both 32-bit and 64-bit modes.

I am heading off to Paris on vacation later today, and will return on
the 19th.  I will commit this when I return.

Peter

Re: [PATCH, rs6000] Modify libgcc's float128 IFUNC resolver functions to use __builtin_cpu_supports()

[Peter Bergner]

On 7/10/17 2:52 PM, Peter Bergner wrote:
> On 7/10/17 9:48 AM, Segher Boessenkool wrote:
>> On Fri, Jul 07, 2017 at 07:14:25PM -0500, Peter Bergner wrote:
>>> On 7/7/17 4:13 PM, Peter Bergner wrote:
>>>> On 7/7/17 10:18 AM, Segher Boessenkool wrote:
>>>>> On Thu, Jul 06, 2017 at 04:21:48PM -0500, Peter Bergner wrote:
>>>>>> 	* config/rs6000/float128-ifunc.c: Don't include auxv.h.
>>>>>> 	(have_ieee_hw_p): Delete function.
>>>>>> 	(SW_OR_HW) Use __builtin_cpu_supports().
>>>>>
>>>>> Okay for trunk.  Thanks!
>>>>
>>>> Given Florian wants this in now to fix a Fedora blocker, I have
>>>> committed this to trunk.  We need the fix on GCC 6 and GCC 5 as
>>>> well, so ok there assuming bootstrapping / regtesting are clean?
>>>
>>> FYI, the bootstrap and regtesting were clean on both the GCC 7 and
>>> GCC 6 release branches.  Ok for those branches?
>>
>> Okay for 7 and 6.  If it is a blocker (or you have another reason you
>> cannot wait to see if regressions pop up), please make sure you test it
>> on many different systems.
> 
> The backports bootstrapped and regtested with no regressions on
> powerpc64le-linux and powerpc64-linux, while running the testsuite
> in both 32-bit and 64-bit modes.
> 
> I am heading off to Paris on vacation later today, and will return on
> the 19th.  I will commit this when I return.

Backports to GCC 7 and GCC 6 release branches have now been committed.

Peter