[powerpc] fpu_control.h: protect use of __builtin_cpu_supports()

Message ID 1562630076-11320-1-git-send-email-pc@us.ibm.com
State Superseded
Headers show
Series
  • [powerpc] fpu_control.h: protect use of __builtin_cpu_supports()
Related show

Commit Message

Paul Clarke July 8, 2019, 11:54 p.m.
From: "Paul A. Clarke" <pc@us.ibm.com>


Using __builtin_cpu_supports() requires support in GCC and Glibc.
My recent patch to fpu_control.h added an unprotected use of
__builtin_cpu_supports(), and this is a user-visible file.

Compilation of Glibc itself will fail with a sufficiently new GCC and
sufficiently old Glibc:
../sysdeps/powerpc/fpu/fegetexcept.c: In function ‘__fegetexcept’:
../sysdeps/powerpc/fpu/fenv_libc.h:52:20: error: builtin ‘__builtin_cpu_supports’ needs GLIBC (2.23 and newer) that exports hardware capability bits [-Werror]

Fixes 3db85a9814784a74536a1f0e7b7ddbfef7dc84bb.

2019-07-08  Paul A. Clarke  <pc@us.ibm.com>

	* sysdeps/powerpc/fpu/fpu_control.h (_FPU_GET_RC): Protect use of
	__builtin_cpu_supports with __BUILTIN_CPU_SUPPORTS__.
---
 sysdeps/powerpc/fpu/fenv_libc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
1.8.3.1

Comments

Florian Weimer July 9, 2019, 6:19 a.m. | #1
* Paul A. Clarke:

> From: "Paul A. Clarke" <pc@us.ibm.com>

>

> Using __builtin_cpu_supports() requires support in GCC and Glibc.

> My recent patch to fpu_control.h added an unprotected use of

> __builtin_cpu_supports(), and this is a user-visible file.


Are you sure?  I don't think it's an installed header, and that's what
“user-visibile file” implies to me.

> Compilation of Glibc itself will fail with a sufficiently new GCC and

> sufficiently old Glibc:

> ../sysdeps/powerpc/fpu/fegetexcept.c: In function ‘__fegetexcept’:

> ../sysdeps/powerpc/fpu/fenv_libc.h:52:20: error: builtin ‘__builtin_cpu_supports’ needs GLIBC (2.23 and newer) that exports hardware capability bits [-Werror]


Presumably there is no way to tell the compiler that the glibc the code
is being built for actually supports the hwcap bits (without rebuilding
the compiler)?

Thanks,
Florian
Andreas Schwab July 9, 2019, 9:50 a.m. | #2
On Jul 09 2019, Florian Weimer <fweimer@redhat.com> wrote:

> * Paul A. Clarke:

>

>> From: "Paul A. Clarke" <pc@us.ibm.com>

>>

>> Using __builtin_cpu_supports() requires support in GCC and Glibc.

>> My recent patch to fpu_control.h added an unprotected use of

>> __builtin_cpu_supports(), and this is a user-visible file.

>

> Are you sure?  I don't think it's an installed header, and that's what

> “user-visibile file” implies to me.


It's in math/Makefile:headers.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Florian Weimer July 9, 2019, 10:07 a.m. | #3
* Andreas Schwab:

> On Jul 09 2019, Florian Weimer <fweimer@redhat.com> wrote:

>

>> * Paul A. Clarke:

>>

>>> From: "Paul A. Clarke" <pc@us.ibm.com>

>>>

>>> Using __builtin_cpu_supports() requires support in GCC and Glibc.

>>> My recent patch to fpu_control.h added an unprotected use of

>>> __builtin_cpu_supports(), and this is a user-visible file.

>>

>> Are you sure?  I don't think it's an installed header, and that's what

>> “user-visibile file” implies to me.

>

> It's in math/Makefile:headers.


fpu_control.h?  Yes.  But the changed file is
sysdeps/powerpc/fpu/fenv_libc.h, which I don't think is installed.

Thanks,
Florian
Paul Clarke July 9, 2019, 2:23 p.m. | #4
On 7/9/19 5:07 AM, Florian Weimer wrote:
> * Andreas Schwab:

>> On Jul 09 2019, Florian Weimer <fweimer@redhat.com> wrote:

>>> * Paul A. Clarke:

>>>>

>>>> Using __builtin_cpu_supports() requires support in GCC and Glibc.

>>>> My recent patch to fpu_control.h added an unprotected use of

>>>> __builtin_cpu_supports(), and this is a user-visible file.

>>>

>>> Are you sure?  I don't think it's an installed header, and that's what

>>> “user-visibile file” implies to me.

>>

>> It's in math/Makefile:headers.

> 

> fpu_control.h?  Yes.  But the changed file is

> sysdeps/powerpc/fpu/fenv_libc.h, which I don't think is installed.


Ugh.  You're both right, of course.  Why I got fpu_control.h stuck in my brain, who knows?

Thank you for your comments.

Let me send a better version of this patch.

PC

Patch

diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
index 55b1697..59a8c44 100644
--- a/sysdeps/powerpc/fpu/fenv_libc.h
+++ b/sysdeps/powerpc/fpu/fenv_libc.h
@@ -47,12 +47,14 @@  extern const fenv_t *__fe_mask_env (void) attribute_hidden;
 
 #ifdef _ARCH_PWR9
 # define fegetenv_status() fegetenv_status_ISA300()
-#else
+#elif defined __BUILTIN_CPU_SUPPORTS__
 # define fegetenv_status()						\
   (__glibc_likely (__builtin_cpu_supports ("arch_3_00"))		\
    ? fegetenv_status_ISA300()						\
    : fegetenv_register()						\
   )
+#else
+# define fegetenv_status() fegetenv_register()
 #endif
 
 /* Equivalent to fesetenv, but takes a fenv_t instead of a pointer.  */