[PATCH] PowerPC - logb[f|l] optimization for POWER7
Ryan S. Arnold
ryan.arnold@gmail.com
Tue May 8 15:45:00 GMT 2012
On Tue, May 8, 2012 at 7:29 AM, Adhemerval Zanella
<azanella@linux.vnet.ibm.com> wrote:
> This patch provides optimized logb (1.2x on PPC32 and 2.5x on PPC64),
> logbf (1.1x on PPC32 and 2.2x on PPC64), and logbl (1.3x on PPC32 and
> 50% on PPC64).
>
> ---
>
> 2012-05-08 Adhemerval Zanella <azanella@linux.vnet.ibm.com>
>
> * sysdeps/powerpc/powerpc32/power7/fpu/s_logb.c: New file: optimized
> logb for POWER7.
> * sysdeps/powerpc/powerpc32/power7/fpu/s_logbf.c: New file: optimized
> logbf for POWER7.
> * sysdeps/powerpc/powerpc32/power7/fpu/s_logbl.c: New file: optimized
> logbl for POWER7.
No colon necessary after "New file:". It should be "New file.
Optimize logb for POWER7".
> * sysdeps/powerpc/powerpc64/power7/fpu/s_logb.c: New file: wrapper for
> the optimized logb for PPC64.
> * sysdeps/powerpc/powerpc64/power7/fpu/s_logbf.c: New file: wrapper for
> the optimized logbf for PPC64.
> * sysdeps/powerpc/powerpc64/power7/fpu/s_logbl.c: New file: wrapper for
> the optimized logbl for PPC64.
I prefer that this mention that it #includes the powerpc32/power7/ .c
file. Something like:
"New file. Use powerpc32/power7/logb[fl].c via #include.
> diff --git a/sysdeps/powerpc/powerpc32/power7/fpu/s_logb.c b/sysdeps/powerpc/powerpc32/power7/fpu/s_logb.c
> new file mode 100644
> index 0000000..5f57c49
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc32/power7/fpu/s_logb.c
> @@ -0,0 +1,74 @@
> +/* logb(). PowerPC/POWER7 version.
> + Copyright (C) 2012 Free Software Foundation, Inc.
> + Contributed by Adhemerval Zanella Netto <azanella@br.ibm.com>.
Roland has indicated that he no longer wants "Contributed by"
statements.. The git log will serve as attribution. This applies to
all files in this patchset.
> + 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include "math_private.h"
> +
> +/* This implementation avoid FP to INT conversions by using VSX bitwise
> + instructions over FP values. */
> +
> +static const double two1div52 = 2.220446049250313e-16; /* 1/2**52 */
> +static const double two10m1 = -1023.0; /* 2**10 -1 */
> +
> +/* FP mask to extract the exponent */
> +static const union {
> + unsigned long long mask;
> + double d;
> +} mask = { 0x7ff0000000000000ULL };
> +
> +double
> +__logb (double x)
> +{
> + double ret;
> +
> + if (__builtin_expect(x == 0.0, 0))
This should have a space between __builtin_expect and the ().
> + return -1.0 / __builtin_fabs (x);
I would like if there were a comment indicating what's going on, i.e.,
/* Raise FE_DIVBYZERO and return -HUGE_VAL[LF]. */
> +
> + /* ret = x & 0x7ff0000000000000; */
> + asm (
> + "xxland %x0,%x1,%x2\n"
> + "fcfid %0,%0"
> + : "=f" (ret)
> + : "f" (x), "f" (mask.d));
> + /* ret = (ret >> 52) - 1023.0; */
> + ret = (ret * two1div52) + two10m1;
> + if (__builtin_expect (ret > -two10m1, 0))
> + return (x * x);
Once again, please comment the special case with an indication of
what's being returned (if it is a special value indicated by the
spec). Please indicate the special value in the comment rather than
just a magic number that represents it. This applies to all files in
this patchset.
> + else if (__builtin_expect (ret == two10m1, 0))
> + {
> + /* POSIX specifies that denormal number is treated as
> + though it were normalized. */
> + int32_t lx, ix;
> + int m1, m2, ma;
> +
> + EXTRACT_WORDS (ix , lx, x);
> + m1 = (ix == 0) ? 0 : __builtin_clz (ix);
> + m2 = (lx == 0) ? 0 : __builtin_clz (lx);
> + ma = (m1 == 0) ? m2 + 32 : m1;
> + return -1022.0 + (double)(11 - ma);
> + }
> + /* Test to avoid logb_downward (0.0) == -0.0 */
> + return ret == -0.0 ? 0.0 : ret;
Is this faster than an builtin abs call?
> +}
> +
> +weak_alias (__logb, logb)
> +
> +#ifdef NO_LONG_DOUBLE
> +strong_alias (__logb, __logbl)
> +weak_alias (__logb, logbl)
> +#endif
> diff --git a/sysdeps/powerpc/powerpc32/power7/fpu/s_logbf.c b/sysdeps/powerpc/powerpc32/power7/fpu/s_logbf.c
> new file mode 100644
> index 0000000..eb276f8
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc32/power7/fpu/s_logbf.c
> @@ -0,0 +1,59 @@
> +/* logbf(). PowerPC/POWER7 version.
> + Copyright (C) 2012 Free Software Foundation, Inc.
> + Contributed by Adhemerval Zanella Netto <azanella@br.ibm.com>.
> + 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include "math_private.h"
> +
> +/* This implementation avoid FP to INT conversions by using VSX bitwise
> + instructions over FP values. */
> +
> +static const double two1div52 = 2.220446049250313e-16; /* 1/2**52 */
> +static const double two10m1 = -1023.0; /* -2**10 + 1 */
> +static const double two7m1 = -127.0; /* -2**7 + 1 */
> +
> +/* FP mask to extract the exponent */
> +static const union {
> + unsigned long long mask;
> + double d;
> +} mask = { 0x7ff0000000000000ULL };
> +
> +float
> +__logbf (float x)
> +{
> + /* VSX operation are all done internally as double */
> + double ret;
> +
> + if (__builtin_expect (x == 0.0, 0))
> + return -1.0 / __builtin_fabsf (x);
> +
> + /* ret = x & 0x7f800000; */
> + asm (
> + "xxland %x0,%x1,%x2\n"
> + "fcfid %0,%0"
> + : "=f"(ret)
> + : "f" (x), "f" (mask.d));
> + /* ret = (ret >> 52) - 1023.0, since ret is double */
> + ret = (ret * two1div52) + two10m1;
> + if (__builtin_expect (ret > -two7m1, 0))
> + return (x * x);
> + /* Since operations are done with double, not need to
> + additional tests for subnormal numbers.
Should be "Since operations are done with double we don't need additional..."
> + The test is to avoid logb_downward (0.0) == -0.0 */
> + return ret == -0.0 ? 0.0 : ret;
> +}
> +weak_alias (__logbf, logbf)
Ryan S. Arnold
More information about the Libc-alpha
mailing list