diff mbox series

[v3,4/7] math: Fix isnanf128 static build

Message ID 20240402140644.2172819-5-adhemerval.zanella@linaro.org
State New
Headers show
Series Fix some libm static issues | expand

Commit Message

Adhemerval Zanella April 2, 2024, 2:06 p.m. UTC 
Some static implementation of float128 routines might call __isnanf128,
which is not provided by the static object.

Checked on x86_64-linux-gnu.
---
 sysdeps/ieee754/float128/float128_private.h | 2 +-
 sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

H.J. Lu May 20, 2024, 4:51 p.m. UTC | #1 
On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> Some static implementation of float128 routines might call __isnanf128,

Which targets do this?

> which is not provided by the static object.
>
> Checked on x86_64-linux-gnu.
> ---
>  sysdeps/ieee754/float128/float128_private.h | 2 +-
>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
> index 38a8bdd0fe..672bf3cccf 100644
> --- a/sysdeps/ieee754/float128/float128_private.h
> +++ b/sysdeps/ieee754/float128/float128_private.h
> @@ -352,7 +352,7 @@
>  #define frexpl frexpf128
>  #define getpayloadl getpayloadf128
>  #define isinfl isinff128_do_not_use
> -#define isnanl isnanf128_do_not_use
> +#define isnanl isnanf128
>  #define ldexpl ldexpf128
>  #define llrintl llrintf128
>  #define llroundl llroundf128
> diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
> index 59f71533ce..b73a4e80d7 100644
> --- a/sysdeps/ieee754/float128/s_isnanf128.c
> +++ b/sysdeps/ieee754/float128/s_isnanf128.c
> @@ -11,7 +11,11 @@
>  #include "../ldbl-128/s_isnanl.c"
>  #if !IS_IN (libm)
>  #include <float128-abi.h>
> +#ifdef SHARED
>  hidden_ver (__isnanf128_impl, __isnanf128)
> +#else
> +strong_alias (__isnanf128_impl, __isnanf128)
> +#endif
>  _weak_alias (__isnanf128_impl, isnanl)
>  versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
>  #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))
> --
> 2.34.1
>
Adhemerval Zanella May 20, 2024, 6:53 p.m. UTC | #2 
On 20/05/24 13:51, H.J. Lu wrote:
> On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> Some static implementation of float128 routines might call __isnanf128,
> 
> Which targets do this?

I will need to recheck all the targets affected by this, but at least 
x86_64 and i686 are. Using the the 'build-math-static-tests=yes' rule
(provided by the first patch in this set):

x86_64-linux-gnu$ make test t=math/test-float128-isnan-static build-math-static-tests=yes
[...]
/home/azanella/toolchain/install/compilers/14/x86_64-linux-gnu/bin/../lib/gcc/x86_64-glibc-linux-gnu/14.1.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/azanella/Projects/glibc/build/x86_64-linux-gnu/math/test-float128-isnan-static.o: in function `isnan_test':
/home/azanella/Projects/glibc/build/x86_64-linux-gnu/math/libm-test-isnan-static.c:56:(.text+0xa9): undefined reference to `__isnanf128'
/home/azanella/toolchain/install/compilers/14/x86_64-linux-gnu/bin/../lib/gcc/x86_64-glibc-linux-gnu/14.1.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/azanella/Projects/glibc/build/x86_64-linux-gnu/math/libm-test-isnan-static.c:56:(.text+0x169): undefined reference to `__isnanf128'
[...]

For aarch64, for instance, the isnan with _Float128 generated a call to
__isnanl which is provided by libc.a:

$ readelf -sW libc.a | grep -w "FUNC.*GLOBAL" | grep __isnanl
    18: 0000000000000000   120 FUNC    GLOBAL HIDDEN     1 __isnanl

With this patch, x86_64 now also provides the __isnanf128:

x86_64-linux-gnu$ readelf -sW libc.a | grep -w "FUNC.*GLOBAL" | grep -w __isnanf128
    12: 0000000000000000    99 FUNC    GLOBAL HIDDEN     1 __isnanf128

> 
>> which is not provided by the static object.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>>  sysdeps/ieee754/float128/float128_private.h | 2 +-
>>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
>> index 38a8bdd0fe..672bf3cccf 100644
>> --- a/sysdeps/ieee754/float128/float128_private.h
>> +++ b/sysdeps/ieee754/float128/float128_private.h
>> @@ -352,7 +352,7 @@
>>  #define frexpl frexpf128
>>  #define getpayloadl getpayloadf128
>>  #define isinfl isinff128_do_not_use
>> -#define isnanl isnanf128_do_not_use
>> +#define isnanl isnanf128
>>  #define ldexpl ldexpf128
>>  #define llrintl llrintf128
>>  #define llroundl llroundf128
>> diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
>> index 59f71533ce..b73a4e80d7 100644
>> --- a/sysdeps/ieee754/float128/s_isnanf128.c
>> +++ b/sysdeps/ieee754/float128/s_isnanf128.c
>> @@ -11,7 +11,11 @@
>>  #include "../ldbl-128/s_isnanl.c"
>>  #if !IS_IN (libm)
>>  #include <float128-abi.h>
>> +#ifdef SHARED
>>  hidden_ver (__isnanf128_impl, __isnanf128)
>> +#else
>> +strong_alias (__isnanf128_impl, __isnanf128)
>> +#endif
>>  _weak_alias (__isnanf128_impl, isnanl)
>>  versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
>>  #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))
>> --
>> 2.34.1
>>
> 
>
H.J. Lu May 20, 2024, 9:34 p.m. UTC | #3 
On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> Some static implementation of float128 routines might call __isnanf128,
> which is not provided by the static object.
>
> Checked on x86_64-linux-gnu.
> ---
>  sysdeps/ieee754/float128/float128_private.h | 2 +-
>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
> index 38a8bdd0fe..672bf3cccf 100644
> --- a/sysdeps/ieee754/float128/float128_private.h
> +++ b/sysdeps/ieee754/float128/float128_private.h
> @@ -352,7 +352,7 @@
>  #define frexpl frexpf128
>  #define getpayloadl getpayloadf128
>  #define isinfl isinff128_do_not_use
> -#define isnanl isnanf128_do_not_use

Why is this change needed?  Will the issue be fixed by

https://patchwork.sourceware.org/project/glibc/list/?series=34121

> +#define isnanl isnanf128
>  #define ldexpl ldexpf128
>  #define llrintl llrintf128
>  #define llroundl llroundf128
> diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
> index 59f71533ce..b73a4e80d7 100644
> --- a/sysdeps/ieee754/float128/s_isnanf128.c
> +++ b/sysdeps/ieee754/float128/s_isnanf128.c
> @@ -11,7 +11,11 @@
>  #include "../ldbl-128/s_isnanl.c"
>  #if !IS_IN (libm)
>  #include <float128-abi.h>
> +#ifdef SHARED
>  hidden_ver (__isnanf128_impl, __isnanf128)
> +#else
> +strong_alias (__isnanf128_impl, __isnanf128)
> +#endif
>  _weak_alias (__isnanf128_impl, isnanl)
>  versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
>  #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))
> --
> 2.34.1
>
Adhemerval Zanella May 21, 2024, 12:32 p.m. UTC | #4 
On 20/05/24 18:34, H.J. Lu wrote:
> On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> Some static implementation of float128 routines might call __isnanf128,
>> which is not provided by the static object.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>>  sysdeps/ieee754/float128/float128_private.h | 2 +-
>>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
>> index 38a8bdd0fe..672bf3cccf 100644
>> --- a/sysdeps/ieee754/float128/float128_private.h
>> +++ b/sysdeps/ieee754/float128/float128_private.h
>> @@ -352,7 +352,7 @@
>>  #define frexpl frexpf128
>>  #define getpayloadl getpayloadf128
>>  #define isinfl isinff128_do_not_use
>> -#define isnanl isnanf128_do_not_use
> 
> Why is this change needed?  Will the issue be fixed by

This is not required indeed, I will remove it.

> 
> https://patchwork.sourceware.org/project/glibc/list/?series=34121

But unfortunately this patch does not fix the missing isnanf128 symbol on libc.a.

> 
>> +#define isnanl isnanf128
>>  #define ldexpl ldexpf128
>>  #define llrintl llrintf128
>>  #define llroundl llroundf128
>> diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
>> index 59f71533ce..b73a4e80d7 100644
>> --- a/sysdeps/ieee754/float128/s_isnanf128.c
>> +++ b/sysdeps/ieee754/float128/s_isnanf128.c
>> @@ -11,7 +11,11 @@
>>  #include "../ldbl-128/s_isnanl.c"
>>  #if !IS_IN (libm)
>>  #include <float128-abi.h>
>> +#ifdef SHARED
>>  hidden_ver (__isnanf128_impl, __isnanf128)
>> +#else
>> +strong_alias (__isnanf128_impl, __isnanf128)
>> +#endif
>>  _weak_alias (__isnanf128_impl, isnanl)
>>  versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
>>  #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))
>> --
>> 2.34.1
>>
> 
>
H.J. Lu May 21, 2024, 12:36 p.m. UTC | #5 
On Tue, May 21, 2024 at 5:32 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 20/05/24 18:34, H.J. Lu wrote:
> > On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >> Some static implementation of float128 routines might call __isnanf128,
> >> which is not provided by the static object.
> >>
> >> Checked on x86_64-linux-gnu.
> >> ---
> >>  sysdeps/ieee754/float128/float128_private.h | 2 +-
> >>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
> >>  2 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
> >> index 38a8bdd0fe..672bf3cccf 100644
> >> --- a/sysdeps/ieee754/float128/float128_private.h
> >> +++ b/sysdeps/ieee754/float128/float128_private.h
> >> @@ -352,7 +352,7 @@
> >>  #define frexpl frexpf128
> >>  #define getpayloadl getpayloadf128
> >>  #define isinfl isinff128_do_not_use
> >> -#define isnanl isnanf128_do_not_use
> >
> > Why is this change needed?  Will the issue be fixed by
>
> This is not required indeed, I will remove it.

Please mention:

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

in the commit log.

> >
> > https://patchwork.sourceware.org/project/glibc/list/?series=34121
>
> But unfortunately this patch does not fix the missing isnanf128 symbol on libc.a.

That is not what I meant.  I thought your isnanl change fixed some
issue you found.

> >
> >> +#define isnanl isnanf128
> >>  #define ldexpl ldexpf128
> >>  #define llrintl llrintf128
> >>  #define llroundl llroundf128
> >> diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
> >> index 59f71533ce..b73a4e80d7 100644
> >> --- a/sysdeps/ieee754/float128/s_isnanf128.c
> >> +++ b/sysdeps/ieee754/float128/s_isnanf128.c
> >> @@ -11,7 +11,11 @@
> >>  #include "../ldbl-128/s_isnanl.c"
> >>  #if !IS_IN (libm)
> >>  #include <float128-abi.h>
> >> +#ifdef SHARED
> >>  hidden_ver (__isnanf128_impl, __isnanf128)
> >> +#else
> >> +strong_alias (__isnanf128_impl, __isnanf128)
> >> +#endif
> >>  _weak_alias (__isnanf128_impl, isnanl)
> >>  versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
> >>  #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))
> >> --
> >> 2.34.1
> >>
> >
> >

Thanks.
Adhemerval Zanella May 21, 2024, 12:56 p.m. UTC | #6 
On 21/05/24 09:36, H.J. Lu wrote:
> On Tue, May 21, 2024 at 5:32 AM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 20/05/24 18:34, H.J. Lu wrote:
>>> On Tue, Apr 2, 2024 at 7:06 AM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>> Some static implementation of float128 routines might call __isnanf128,
>>>> which is not provided by the static object.
>>>>
>>>> Checked on x86_64-linux-gnu.
>>>> ---
>>>>  sysdeps/ieee754/float128/float128_private.h | 2 +-
>>>>  sysdeps/ieee754/float128/s_isnanf128.c      | 4 ++++
>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
>>>> index 38a8bdd0fe..672bf3cccf 100644
>>>> --- a/sysdeps/ieee754/float128/float128_private.h
>>>> +++ b/sysdeps/ieee754/float128/float128_private.h
>>>> @@ -352,7 +352,7 @@
>>>>  #define frexpl frexpf128
>>>>  #define getpayloadl getpayloadf128
>>>>  #define isinfl isinff128_do_not_use
>>>> -#define isnanl isnanf128_do_not_use
>>>
>>> Why is this change needed?  Will the issue be fixed by
>>
>> This is not required indeed, I will remove it.
> 
> Please mention:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=31774
> 
> in the commit log.

Ack.

> 
>>>
>>> https://patchwork.sourceware.org/project/glibc/list/?series=34121
>>
>> But unfortunately this patch does not fix the missing isnanf128 symbol on libc.a.
> 
> That is not what I meant.  I thought your isnanl change fixed some
> issue you found.

Right, to make it clear your patch https://patchwork.sourceware.org/project/glibc/list/?series=34121
does not fix the missing isnanf128 required for static build of math tests.  This is
fixed by this patch.
diff mbox series

Patch

diff --git a/sysdeps/ieee754/float128/float128_private.h b/sysdeps/ieee754/float128/float128_private.h
index 38a8bdd0fe..672bf3cccf 100644
--- a/sysdeps/ieee754/float128/float128_private.h
+++ b/sysdeps/ieee754/float128/float128_private.h
@@ -352,7 +352,7 @@ 
 #define frexpl frexpf128
 #define getpayloadl getpayloadf128
 #define isinfl isinff128_do_not_use
-#define isnanl isnanf128_do_not_use
+#define isnanl isnanf128
 #define ldexpl ldexpf128
 #define llrintl llrintf128
 #define llroundl llroundf128
diff --git a/sysdeps/ieee754/float128/s_isnanf128.c b/sysdeps/ieee754/float128/s_isnanf128.c
index 59f71533ce..b73a4e80d7 100644
--- a/sysdeps/ieee754/float128/s_isnanf128.c
+++ b/sysdeps/ieee754/float128/s_isnanf128.c
@@ -11,7 +11,11 @@ 
 #include "../ldbl-128/s_isnanl.c"
 #if !IS_IN (libm)
 #include <float128-abi.h>
+#ifdef SHARED
 hidden_ver (__isnanf128_impl, __isnanf128)
+#else
+strong_alias (__isnanf128_impl, __isnanf128)
+#endif
 _weak_alias (__isnanf128_impl, isnanl)
 versioned_symbol (libc, __isnanf128_impl, __isnanf128, GLIBC_2_34);
 #if (SHLIB_COMPAT (libc, FLOAT128_VERSION_M, GLIBC_2_34))