| Message ID | 20201215141339.2684384-3-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | x86 pseudo-normal numbers | expand |
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
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) 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. */ + 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; +}