[v2] New generic cosf

Message ID 7aa9f1c0-cced-d24e-8982-4cdb346f1a6d@us.ibm.com
State Dropped
Headers

Commit Message

Paul A. Clarke Dec. 6, 2017, 6:47 p.m. UTC
  The same logic used in s_cosf.S version for x86 and powerpc
is used to create a generic s_cosf.c, so there is no performance
improvement in x86_64 and powerpc64.

-- 8< --
For s390, this is the improvement noted.

Before patch:
  "cosf": {
   "": {
    "duration": 1.00388e+10,
    "iterations": 1.34624e+08,
    "max": 1655.32,
    "min": 4.243,
    "mean": 74.5692
   }
  }
After patch:
  "cosf": {
   "": {
    "duration": 9.89888e+09,
    "iterations": 4.63972e+08,
    "max": 1089.59,
    "min": 6.789,
    "mean": 21.3351
   }
  }

Tested on s390x, x86_64 and powerpc64le and powerpc32.

I'll reply to this post with a diff with the latest generic s_sinf.c,
since including it here could easily be confused with the patch body.
-- 8< --

2017-12-06  Paul A. Clarke  <pc@us.ibm.com>

	* sysdeps/ieee754/flt-32/s_cosf.c: New implementation.
---
 sysdeps/ieee754/flt-32/s_cosf.c | 255 +++++++++++++++++++++++++++++++++-------
 1 file changed, 215 insertions(+), 40 deletions(-)
  

Comments

H.J. Lu Dec. 6, 2017, 7:14 p.m. UTC | #1
On Wed, Dec 6, 2017 at 10:47 AM, Paul Clarke <pc@us.ibm.com> wrote:
> The same logic used in s_cosf.S version for x86 and powerpc
> is used to create a generic s_cosf.c, so there is no performance
> improvement in x86_64 and powerpc64.
>
> -- 8< --
> For s390, this is the improvement noted.
>
> Before patch:
>   "cosf": {
>    "": {
>     "duration": 1.00388e+10,
>     "iterations": 1.34624e+08,
>     "max": 1655.32,
>     "min": 4.243,
>     "mean": 74.5692
>    }
>   }
> After patch:
>   "cosf": {
>    "": {
>     "duration": 9.89888e+09,
>     "iterations": 4.63972e+08,
>     "max": 1089.59,
>     "min": 6.789,
>     "mean": 21.3351
>    }
>   }
>
> Tested on s390x, x86_64 and powerpc64le and powerpc32.
>
> I'll reply to this post with a diff with the latest generic s_sinf.c,
> since including it here could easily be confused with the patch body.

Please provide a diff against the latest generic s_sinf.c.

I assume both s_sinf.c and s_cosf.c use the same set of constant
tables.  We should share them instead of duplication.
  
Joseph Myers Dec. 7, 2017, 12:09 a.m. UTC | #2
On Wed, 6 Dec 2017, Paul Clarke wrote:

> +/* PI/2 with 98 bits of accuracy.  */
> +static const double PI_2_hi = -0x1.921fb544p+0;
> +static const double PI_2_lo = -0x1.0b4611a626332p-34;

This is actually -PI/2, not PI/2.  The comment in s_sinf.c should be fixed 
accordingly.

This cosf implementation is OK with that change.
  
Joseph Myers Dec. 7, 2017, 12:12 a.m. UTC | #3
On Wed, 6 Dec 2017, H.J. Lu wrote:

> I assume both s_sinf.c and s_cosf.c use the same set of constant
> tables.  We should share them instead of duplication.

These implementations were originally proposed as an optimized sincosf.  I 
suggested in <https://sourceware.org/ml/libc-alpha/2017-10/msg00366.html> 
that a single source file for all three functions might well make sense, 
in which case the constants would naturally be shared at that point.
  
Andreas Schwab Dec. 7, 2017, 10:36 a.m. UTC | #4
On Dez 07 2017, Joseph Myers <joseph@codesourcery.com> wrote:

> On Wed, 6 Dec 2017, Paul Clarke wrote:
>
>> +/* PI/2 with 98 bits of accuracy.  */
>> +static const double PI_2_hi = -0x1.921fb544p+0;
>> +static const double PI_2_lo = -0x1.0b4611a626332p-34;
>
> This is actually -PI/2, not PI/2.  The comment in s_sinf.c should be fixed 
> accordingly.

And the variable should be renamed to reflect that.

Andreas.
  

Patch

diff --git a/sysdeps/ieee754/flt-32/s_cosf.c b/sysdeps/ieee754/flt-32/s_cosf.c
index 5ed0bca..bf83e09 100644
--- a/sysdeps/ieee754/flt-32/s_cosf.c
+++ b/sysdeps/ieee754/flt-32/s_cosf.c
@@ -1,21 +1,20 @@ 
-/* s_cosf.c -- float version of s_cos.c.
- * Conversion to float by Ian Lance Taylor, Cygnus Support, ian@cygnus.com.
- */
-
-/*
- * ====================================================
- * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
- *
- * Developed at SunPro, a Sun Microsystems, Inc. business.
- * Permission to use, copy, modify, and distribute this
- * software is freely granted, provided that this notice
- * is preserved.
- * ====================================================
- */
-
-#if defined(LIBM_SCCS) && !defined(lint)
-static char rcsid[] = "$NetBSD: s_cosf.c,v 1.4 1995/05/10 20:47:03 jtc Exp $";
-#endif
+/* Compute cosine of argument.
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
 #include <math.h>
@@ -28,35 +27,211 @@  static char rcsid[] = "$NetBSD: s_cosf.c,v 1.4 1995/05/10 20:47:03 jtc Exp $";
 # define COSF_FUNC COSF
 #endif
 
-float COSF_FUNC(float x)
+/* Chebyshev constants for cos, range -PI/4 - PI/4.  */
+static const double C0 = -0x1.ffffffffe98aep-2;
+static const double C1 =  0x1.55555545c50c7p-5;
+static const double C2 = -0x1.6c16b348b6874p-10;
+static const double C3 =  0x1.a00eb9ac43ccp-16;
+static const double C4 = -0x1.23c97dd8844d7p-22;
+
+/* Chebyshev constants for sin, range -PI/4 - PI/4.  */
+static const double S0 = -0x1.5555555551cd9p-3;
+static const double S1 =  0x1.1111110c2688bp-7;
+static const double S2 = -0x1.a019f8b4bd1f9p-13;
+static const double S3 =  0x1.71d7264e6b5b4p-19;
+static const double S4 = -0x1.a947e1674b58ap-26;
+
+/* Chebyshev constants for cos, range 2^-27 - 2^-5.  */
+static const double CC0 = -0x1.fffffff5cc6fdp-2;
+static const double CC1 =  0x1.55514b178dac5p-5;
+
+/* PI/2 with 98 bits of accuracy.  */
+static const double PI_2_hi = -0x1.921fb544p+0;
+static const double PI_2_lo = -0x1.0b4611a626332p-34;
+
+static const double inv_PI_4 = 0x1.45f306dc9c883p+0; /* 4/PI.  */
+
+#define FLOAT_EXPONENT_SHIFT 23
+#define FLOAT_EXPONENT_BIAS 127
+
+static const double pio2_table[] = {
+  0 * M_PI_2,
+  1 * M_PI_2,
+  2 * M_PI_2,
+  3 * M_PI_2,
+  4 * M_PI_2,
+  5 * M_PI_2
+};
+
+static const double invpio4_table[] = {
+  0x0p+0,
+  0x1.45f306cp+0,
+  0x1.c9c882ap-28,
+  0x1.4fe13a8p-58,
+  0x1.f47d4dp-85,
+  0x1.bb81b6cp-112,
+  0x1.4acc9ep-142,
+  0x1.0e4107cp-169
+};
+
+static const double ones[] = { 1.0, -1.0 };
+
+
+/* Compute the cosine value using Chebyshev polynomials where
+   THETA is the range reduced absolute value of the input
+   and it is less than Pi/4,
+   N is calculated as trunc(|x|/(Pi/4)) + 1 and it is used to decide
+   whether a sine or cosine approximation is more accurate and
+   the sign of the result.  */
+static inline float
+reduced (double theta, unsigned int n)
 {
-	float y[2],z=0.0;
-	int32_t n,ix;
+  double sign, cx;
+  const double theta2 = theta * theta;
 
-	GET_FLOAT_WORD(ix,x);
+  /* Determine positive or negative primary interval.  */
+  n += 2;
+  sign = ones[(n >> 2) & 1];
 
-    /* |x| ~< pi/4 */
-	ix &= 0x7fffffff;
-	if(ix <= 0x3f490fd8) return __kernel_cosf(x,z);
+  /* Are we in the primary interval of sin or cos?  */
+  if ((n & 2) == 0)
+    {
+      /* Here cosf() is calculated using sin Chebyshev polynomial:
+	x+x^3*(S0+x^2*(S1+x^2*(S2+x^2*(S3+x^2*S4)))).  */
+      cx = S3 + theta2 * S4;
+      cx = S2 + theta2 * cx;
+      cx = S1 + theta2 * cx;
+      cx = S0 + theta2 * cx;
+      cx = theta + theta * theta2 * cx;
+    }
+  else
+    {
+     /* Here cosf() is calculated using cos Chebyshev polynomial:
+	1.0+x^2*(C0+x^2*(C1+x^2*(C2+x^2*(C3+x^2*C4)))).  */
+      cx = C3 + theta2 * C4;
+      cx = C2 + theta2 * cx;
+      cx = C1 + theta2 * cx;
+      cx = C0 + theta2 * cx;
+      cx = 1. + theta2 * cx;
+    }
+  return sign * cx;
+}
 
-    /* cos(Inf or NaN) is NaN */
-	else if (ix>=0x7f800000) {
-	  if (ix == 0x7f800000)
-	    __set_errno (EDOM);
-	  return x-x;
+float
+COSF_FUNC (float x)
+{
+  double theta = x;
+  double abstheta = fabs (theta);
+  if (isless (abstheta, M_PI_4))
+    {
+      double cx;
+      if (abstheta >= 0x1p-5)
+	{
+	  const double theta2 = theta * theta;
+	  /* Chebyshev polynomial of the form for cos:
+	   * 1 + x^2 (C0 + x^2 (C1 + x^2 (C2 + x^2 (C3 + x^2 * C4)))).  */
+	  cx = C3 + theta2 * C4;
+	  cx = C2 + theta2 * cx;
+	  cx = C1 + theta2 * cx;
+	  cx = C0 + theta2 * cx;
+	  cx = 1. + theta2 * cx;
+	  return cx;
 	}
-
-    /* argument reduction needed */
-	else {
-	    n = __ieee754_rem_pio2f(x,y);
-	    switch(n&3) {
-		case 0: return  __kernel_cosf(y[0],y[1]);
-		case 1: return -__kernel_sinf(y[0],y[1],1);
-		case 2: return -__kernel_cosf(y[0],y[1]);
-		default:
-		        return  __kernel_sinf(y[0],y[1],1);
+      else if (abstheta >= 0x1p-27)
+	{
+	  /* A simpler Chebyshev approximation is close enough for this range:
+	   * 1 + x^2 (CC0 + x^3 * CC1).  */
+	  const double theta2 = theta * theta;
+	  cx = CC0 + theta * theta2 * CC1;
+	  cx = 1.0 + theta2 * cx;
+	  return cx;
+	}
+      else
+	{
+	  /* For small enough |theta|, this is close enough.  */
+	  return 1.0 - abstheta;
+	}
+    }
+  else /* |theta| >= Pi/4.  */
+    {
+      if (isless (abstheta, 9 * M_PI_4))
+	{
+	  /* There are cases where FE_UPWARD rounding mode can
+	     produce a result of abstheta * inv_PI_4 == 9,
+	     where abstheta < 9pi/4, so the domain for
+	     pio2_table must go to 5 (9 / 2 + 1).  */
+	  unsigned int n = (abstheta * inv_PI_4) + 1;
+	  theta = abstheta - pio2_table[n / 2];
+	  return reduced (theta, n);
+	}
+      else if (isless (abstheta, INFINITY))
+	{
+	  if (abstheta < 0x1p+23)
+	    {
+	      unsigned int n = ((unsigned int) (abstheta * inv_PI_4)) + 1;
+	      double x = n / 2;
+	      theta = x * PI_2_lo + (x * PI_2_hi + abstheta);
+	      /* Argument reduction needed.  */
+	      return reduced (theta, n);
 	    }
+	  else /* |theta| >= 2^23.  */
+	    {
+	      x = fabsf (x);
+	      int exponent;
+	      GET_FLOAT_WORD (exponent, x);
+	      exponent = (exponent >> FLOAT_EXPONENT_SHIFT)
+			 - FLOAT_EXPONENT_BIAS;
+	      exponent += 3;
+	      exponent /= 28;
+	      double a = invpio4_table[exponent] * x;
+	      double b = invpio4_table[exponent + 1] * x;
+	      double c = invpio4_table[exponent + 2] * x;
+	      double d = invpio4_table[exponent + 3] * x;
+	      uint64_t l = a;
+	      l &= ~0x7;
+	      a -= l;
+	      double e = a + b;
+	      l = e;
+	      e = a - l;
+	      if (l & 1)
+		{
+		  e -= 1.0;
+		  e += b;
+		  e += c;
+		  e += d;
+		  e *= M_PI_4;
+		  return reduced (e, l + 1);
+		}
+	      else
+		{
+		  e += b;
+		  e += c;
+		  e += d;
+		  if (e <= 1.0)
+		    {
+		      e *= M_PI_4;
+		      return reduced (e, l + 1);
+		    }
+		  else
+		    {
+		      l++;
+		      e -= 2.0;
+		      e *= M_PI_4;
+		      return reduced (e, l + 1);
+		    }
+		}
+	    }
+	}
+      else
+	{
+	  int32_t ix;
+	  GET_FLOAT_WORD (ix, abstheta);
+	  /* cos(Inf or NaN) is NaN.  */
+	  if (ix == 0x7f800000) /* Inf.  */
+	    __set_errno (EDOM);
+	  return x - x;
 	}
+    }
 }
 
 #ifndef COSF