[2/5] x86 long double: Support pseudo numbers in isnanl
Commit Message
Sync up with gcc behaviour. This change splits out the core isnanl
logic so that it can be reused.
---
sysdeps/i386/fpu/s_isnanl.c | 10 ++--------
sysdeps/x86/fpu/isnanl_common.h | 32 ++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 8 deletions(-)
create mode 100644 sysdeps/x86/fpu/isnanl_common.h
Comments
On 15/12/2020 11:13, Siddhesh Poyarekar via Libc-alpha wrote:
> Sync up with gcc behaviour. This change splits out the core isnanl
> logic so that it can be reused.
> ---
> sysdeps/i386/fpu/s_isnanl.c | 10 ++--------
> sysdeps/x86/fpu/isnanl_common.h | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 8 deletions(-)
> create mode 100644 sysdeps/x86/fpu/isnanl_common.h
>
> diff --git a/sysdeps/i386/fpu/s_isnanl.c b/sysdeps/i386/fpu/s_isnanl.c
> index fb97317bc9..6824f31d96 100644
> --- a/sysdeps/i386/fpu/s_isnanl.c
> +++ b/sysdeps/i386/fpu/s_isnanl.c
> @@ -25,19 +25,13 @@ static char rcsid[] = "$NetBSD: $";
>
> #include <math.h>
> #include <math_private.h>
> +#include "sysdeps/x86/fpu/isnanl_common.h"
>
> int __isnanl(long double x)
> {
> int32_t se,hx,lx;
> GET_LDOUBLE_WORDS(se,hx,lx,x);
> - se = (se & 0x7fff) << 1;
> - /* The additional & 0x7fffffff is required because Intel's
> - extended format has the normally implicit 1 explicit
> - present. Sigh! */
> - lx |= hx & 0x7fffffff;
> - se |= (uint32_t)(lx|(-lx))>>31;
> - se = 0xfffe - se;
> - return (int)((uint32_t)(se))>>16;
> + return x86_isnanl (se, hx, lx);
> }
> hidden_def (__isnanl)
> weak_alias (__isnanl, isnanl)
As for fpclassify, I think a better strategy would to move this code to
sysdeps/x86/fpu.
> diff --git a/sysdeps/x86/fpu/isnanl_common.h b/sysdeps/x86/fpu/isnanl_common.h
> new file mode 100644
> index 0000000000..60a4af5b41
> --- /dev/null
> +++ b/sysdeps/x86/fpu/isnanl_common.h
> @@ -0,0 +1,32 @@
> +/* Common inline isnanl implementation for x86.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +static inline __always_inline int
> +x86_isnanl (int32_t se, int32_t hx, int32_t lx)
> +{
> + se = (se & 0x7fff) << 1;
> + /* Detect pseudo-normal numbers, i.e. exponent is non-zero and the top
> + bit of the significand is not set. */
This sentence sounds strange, would 'if' instead of 'of' be better?
> + int pn = (uint32_t)((~hx & 0x80000000) & (se | (-se))) >> 31;
Ok.
> + /* Clear the significand bit when computing mantissa. */
> + lx |= hx & 0x7fffffff;
> + se |= (uint32_t)(lx | (-lx)) >> 31;
> + se = 0xfffe - se;
> +
> + return (int)(((uint32_t)(se)) >> 16) | pn;
> +}
>
Not sure if this is really a gain here, is this routine really worth
to move to inline instead of just calling the __isnanl (since it seems
to be used solely on issignalingl)?
On 12/23/20 12:34 AM, Adhemerval Zanella via Libc-alpha wrote:
>
>
> On 15/12/2020 11:13, Siddhesh Poyarekar via Libc-alpha wrote:
>> Sync up with gcc behaviour. This change splits out the core isnanl
>> logic so that it can be reused.
>> ---
>> sysdeps/i386/fpu/s_isnanl.c | 10 ++--------
>> sysdeps/x86/fpu/isnanl_common.h | 32 ++++++++++++++++++++++++++++++++
>> 2 files changed, 34 insertions(+), 8 deletions(-)
>> create mode 100644 sysdeps/x86/fpu/isnanl_common.h
>>
>> diff --git a/sysdeps/i386/fpu/s_isnanl.c b/sysdeps/i386/fpu/s_isnanl.c
>> index fb97317bc9..6824f31d96 100644
>> --- a/sysdeps/i386/fpu/s_isnanl.c
>> +++ b/sysdeps/i386/fpu/s_isnanl.c
>> @@ -25,19 +25,13 @@ static char rcsid[] = "$NetBSD: $";
>>
>> #include <math.h>
>> #include <math_private.h>
>> +#include "sysdeps/x86/fpu/isnanl_common.h"
>>
>> int __isnanl(long double x)
>> {
>> int32_t se,hx,lx;
>> GET_LDOUBLE_WORDS(se,hx,lx,x);
>> - se = (se & 0x7fff) << 1;
>> - /* The additional & 0x7fffffff is required because Intel's
>> - extended format has the normally implicit 1 explicit
>> - present. Sigh! */
>> - lx |= hx & 0x7fffffff;
>> - se |= (uint32_t)(lx|(-lx))>>31;
>> - se = 0xfffe - se;
>> - return (int)((uint32_t)(se))>>16;
>> + return x86_isnanl (se, hx, lx);
>> }
>> hidden_def (__isnanl)
>> weak_alias (__isnanl, isnanl)
>
> As for fpclassify, I think a better strategy would to move this code to
> sysdeps/x86/fpu.
OK.
>
>> diff --git a/sysdeps/x86/fpu/isnanl_common.h b/sysdeps/x86/fpu/isnanl_common.h
>> new file mode 100644
>> index 0000000000..60a4af5b41
>> --- /dev/null
>> +++ b/sysdeps/x86/fpu/isnanl_common.h
>> @@ -0,0 +1,32 @@
>> +/* Common inline isnanl implementation for x86.
>> + Copyright (C) 2020 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <https://www.gnu.org/licenses/>. */
>> +
>> +static inline __always_inline int
>> +x86_isnanl (int32_t se, int32_t hx, int32_t lx)
>> +{
>> + se = (se & 0x7fff) << 1;
>> + /* Detect pseudo-normal numbers, i.e. exponent is non-zero and the top
>> + bit of the significand is not set. */
>
> This sentence sounds strange, would 'if' instead of 'of' be better?
>
Yes that's a typo :)
>> + int pn = (uint32_t)((~hx & 0x80000000) & (se | (-se))) >> 31;
>
> Ok.
>
>> + /* Clear the significand bit when computing mantissa. */
>> + lx |= hx & 0x7fffffff;
>> + se |= (uint32_t)(lx | (-lx)) >> 31;
>> + se = 0xfffe - se;
>> +
>> + return (int)(((uint32_t)(se)) >> 16) | pn;
>> +}
>>
>
> Not sure if this is really a gain here, is this routine really worth
> to move to inline instead of just calling the __isnanl (since it seems
> to be used solely on issignalingl)?
>
The functions are micro-optimised to even avoid branches, so I reckon
the extra indirection would be a no no. Also, the inlining ought to
optimize issignaling further as there are redundant ops in there.
Siddhesh
On 12/23/20 7:19 AM, Siddhesh Poyarekar via Libc-alpha wrote:
>>> + /* Detect pseudo-normal numbers, i.e. exponent is non-zero and the
>>> top
>>> + bit of the significand is not set. */
>>
>> This sentence sounds strange, would 'if' instead of 'of' be better?
>>
>
> Yes that's a typo :)
>
Oh wait, it's not. "the top bit of the significant is not set", i.e.
the MSB of the significand.
Siddhesh
@@ -25,19 +25,13 @@ static char rcsid[] = "$NetBSD: $";
#include <math.h>
#include <math_private.h>
+#include "sysdeps/x86/fpu/isnanl_common.h"
int __isnanl(long double x)
{
int32_t se,hx,lx;
GET_LDOUBLE_WORDS(se,hx,lx,x);
- se = (se & 0x7fff) << 1;
- /* The additional & 0x7fffffff is required because Intel's
- extended format has the normally implicit 1 explicit
- present. Sigh! */
- lx |= hx & 0x7fffffff;
- se |= (uint32_t)(lx|(-lx))>>31;
- se = 0xfffe - se;
- return (int)((uint32_t)(se))>>16;
+ return x86_isnanl (se, hx, lx);
}
hidden_def (__isnanl)
weak_alias (__isnanl, isnanl)
new file mode 100644
@@ -0,0 +1,32 @@
+/* Common inline isnanl implementation for x86.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+static inline __always_inline int
+x86_isnanl (int32_t se, int32_t hx, int32_t lx)
+{
+ se = (se & 0x7fff) << 1;
+ /* Detect pseudo-normal numbers, i.e. exponent is non-zero and the top
+ bit of the significand is not set. */
+ int pn = (uint32_t)((~hx & 0x80000000) & (se | (-se))) >> 31;
+ /* Clear the significand bit when computing mantissa. */
+ lx |= hx & 0x7fffffff;
+ se |= (uint32_t)(lx | (-lx)) >> 31;
+ se = 0xfffe - se;
+
+ return (int)(((uint32_t)(se)) >> 16) | pn;
+}