[powerpc] fpu_control.h: protect use of __builtin_cpu_supports()
Commit Message
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(-)
Comments
* 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
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:
> 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
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
@@ -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. */