| Message ID | bf94613b3c06a1a165c4e3818f09a8ea5d477df8.1592254374.git.murphyp@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv4,1/2] powerpc64le: refactor e_sqrtf128.c | expand |
On Mon, Jun 15, 2020 at 03:59:06PM -0500, Paul E. Murphy via Libc-alpha wrote: > Combine both implementations into a single file to allow > building twice with appropriate multiarch support when possible. > Notably, gcc7x does not supporting inlining sqrtf128 via the usual > __builtin_sqrtf128, it arrived in gcc8. Not sure you need that last sentence. > Likewise, this maintains the existing optimal behavior when > building with a power9 or newer cpu. You may not need this sentence either. It's pretty clear in the code. > --- > sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c | 8 +++- > .../powerpc64/le/power9/fpu/e_sqrtf128.c | 38 ------------------- > 2 files changed, 7 insertions(+), 39 deletions(-) > delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c > > diff --git a/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c b/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c > index 950bdad54a..fac8def02a 100644 > --- a/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c > +++ b/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c > @@ -26,6 +26,7 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <float128_private.h> > #include <math.h> > #include <libm-alias-finite.h> > > @@ -42,16 +43,21 @@ > __float128 > __ieee754_sqrtf128 (__float128 a) > { > + __float128 r; > + /* For multiarch builds, this will be built twice. */ Likewise, you may not need this comment. If you decide to keep it, I'd move it up and out of the function. Otherwise, it seems to apply just to the code below. > +#ifndef _ARCH_PWR9 > FP_DECL_EX; > FP_DECL_Q (A); > FP_DECL_Q (R); > - __float128 r; > > FP_INIT_ROUNDMODE; > FP_UNPACK_Q (A, a); > FP_SQRT_Q (R, A); > FP_PACK_Q (r, R); > FP_HANDLE_EXCEPTIONS; > +#else > + asm ("xssqrtqp %0,%1" : "=v" (r) : "v" (a)); > +#endif > return r; > } > libm_alias_finite (__ieee754_sqrtf128, __sqrtf128) > diff --git a/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c b/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c > deleted file mode 100644 > index 232fc773c3..0000000000 > --- a/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c > +++ /dev/null > @@ -1,38 +0,0 @@ > -/* POWER9 sqrt for _Float128 > - Return sqrt(a) > - Copyright (C) 2017-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. > - > - In addition to the permissions in the GNU Lesser General Public > - License, the Free Software Foundation gives you unlimited > - permission to link the compiled version of this file into > - combinations with other programs, and to distribute those > - combinations without any restriction coming from the use of this > - file. (The Lesser General Public License restrictions do apply in > - other respects; for example, they cover modification of the file, > - and distribution when not linked into a combine executable.) > - > - 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/>. */ > - > -#include <libm-alias-finite.h> > - > -__float128 > -__ieee754_sqrtf128 (__float128 a) > -{ > - __float128 z; > - asm ("xssqrtqp %0,%1" : "=v" (z) : "v" (a)); > - return z; > -} > -libm_alias_finite (__ieee754_sqrtf128, __sqrtf128) > -- With those nits addressed, or not, LGTM. PC
On 6/15/20 8:33 PM, Paul A. Clarke wrote: > On Mon, Jun 15, 2020 at 03:59:06PM -0500, Paul E. Murphy via Libc-alpha wrote: >> Combine both implementations into a single file to allow >> building twice with appropriate multiarch support when possible. >> Notably, gcc7x does not supporting inlining sqrtf128 via the usual >> __builtin_sqrtf128, it arrived in gcc8. > > Not sure you need that last sentence. > >> Likewise, this maintains the existing optimal behavior when >> building with a power9 or newer cpu. > > You may not need this sentence either. It's pretty clear in the code. I've removed both, and pushed. Thank you for your review.
diff --git a/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c b/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c index 950bdad54a..fac8def02a 100644 --- a/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c +++ b/sysdeps/powerpc/powerpc64/le/fpu/e_sqrtf128.c @@ -26,6 +26,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <float128_private.h> #include <math.h> #include <libm-alias-finite.h> @@ -42,16 +43,21 @@ __float128 __ieee754_sqrtf128 (__float128 a) { + __float128 r; + /* For multiarch builds, this will be built twice. */ +#ifndef _ARCH_PWR9 FP_DECL_EX; FP_DECL_Q (A); FP_DECL_Q (R); - __float128 r; FP_INIT_ROUNDMODE; FP_UNPACK_Q (A, a); FP_SQRT_Q (R, A); FP_PACK_Q (r, R); FP_HANDLE_EXCEPTIONS; +#else + asm ("xssqrtqp %0,%1" : "=v" (r) : "v" (a)); +#endif return r; } libm_alias_finite (__ieee754_sqrtf128, __sqrtf128) diff --git a/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c b/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c deleted file mode 100644 index 232fc773c3..0000000000 --- a/sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c +++ /dev/null @@ -1,38 +0,0 @@ -/* POWER9 sqrt for _Float128 - Return sqrt(a) - Copyright (C) 2017-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. - - In addition to the permissions in the GNU Lesser General Public - License, the Free Software Foundation gives you unlimited - permission to link the compiled version of this file into - combinations with other programs, and to distribute those - combinations without any restriction coming from the use of this - file. (The Lesser General Public License restrictions do apply in - other respects; for example, they cover modification of the file, - and distribution when not linked into a combine executable.) - - 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/>. */ - -#include <libm-alias-finite.h> - -__float128 -__ieee754_sqrtf128 (__float128 a) -{ - __float128 z; - asm ("xssqrtqp %0,%1" : "=v" (z) : "v" (a)); - return z; -} -libm_alias_finite (__ieee754_sqrtf128, __sqrtf128)