[4/4] Use copysign instead of ternary for some sin/cos input ranges

Message ID 1474998553-2366-5-git-send-email-siddhesh@sourceware.org
State Committed
Headers

Commit Message

Siddhesh Poyarekar Sept. 27, 2016, 5:49 p.m. UTC
  These are remaining cases where we can deduce and conclude that the
sign of the result should be the same as the sign of the input being
checked.  For example, for sin(x), the sign of the result is the same
as the result itself for x < pi.  Likewise, for sine values where x
after range reduction falls into this range and its sign is preserved.

	* sysdeps/ieee754/dbl-64/s_sin.c (do_sincos_1): Use copysign
	instead of ternary condition.
	(do_sincos_2): Likewise.
	(__sin): Likewise.
	(__cos): Likewise.
	(slow): Likewise.
	(sloww): Likewise.
	(sloww1): Likewise.
	(bsloww): Likewise.
	(bsloww1): Likewise.
---
 sysdeps/ieee754/dbl-64/s_sin.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
  

Comments

Manfred Sept. 28, 2016, 12:59 p.m. UTC | #1
On 9/27/2016 7:49 PM, Siddhesh Poyarekar wrote:
> @@ -479,7 +479,7 @@ __sin (double x)
>      {
>        res = do_sin (x, 0, &cor);
>        retval = (res == res + 1.096 * cor) ? res : slow1 (x);
> -      retval = m > 0 ? retval : -retval;
> +      retval = __copysign (retval, x);
>      }				/*   else  if (k < 0x3feb6000)    */
>
The ternary uses m, __copysign uses x. It could be correct (I didn't 
check the full context), but just in case...
  
Carlos O'Donell Sept. 28, 2016, 8:04 p.m. UTC | #2
On 09/28/2016 08:59 AM, Manfred wrote:
> 
> 
> On 9/27/2016 7:49 PM, Siddhesh Poyarekar wrote:
>> @@ -479,7 +479,7 @@ __sin (double x)
>>      {
>>        res = do_sin (x, 0, &cor);
>>        retval = (res == res + 1.096 * cor) ? res : slow1 (x);
>> -      retval = m > 0 ? retval : -retval;
>> +      retval = __copysign (retval, x);
>>      }                /*   else  if (k < 0x3feb6000)    */
>>
> The ternary uses m, __copysign uses x. It could be correct (I didn't check the full context), but just in case...

In both those cases X < pi, and therefore the result has the same sign as X.
  
Carlos O'Donell Sept. 28, 2016, 8:09 p.m. UTC | #3
On 09/27/2016 01:49 PM, Siddhesh Poyarekar wrote:
> These are remaining cases where we can deduce and conclude that the
> sign of the result should be the same as the sign of the input being
> checked.  For example, for sin(x), the sign of the result is the same
> as the result itself for x < pi.  Likewise, for sine values where x
> after range reduction falls into this range and its sign is preserved.
> 
> 	* sysdeps/ieee754/dbl-64/s_sin.c (do_sincos_1): Use copysign
> 	instead of ternary condition.
> 	(do_sincos_2): Likewise.
> 	(__sin): Likewise.
> 	(__cos): Likewise.
> 	(slow): Likewise.
> 	(sloww): Likewise.
> 	(sloww1): Likewise.
> 	(bsloww): Likewise.
> 	(bsloww1): Likewise.

LGTM.

> ---
>  sysdeps/ieee754/dbl-64/s_sin.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/sysdeps/ieee754/dbl-64/s_sin.c b/sysdeps/ieee754/dbl-64/s_sin.c
> index e4333a4..40d538d 100644
> --- a/sysdeps/ieee754/dbl-64/s_sin.c
> +++ b/sysdeps/ieee754/dbl-64/s_sin.c
> @@ -344,7 +344,7 @@ do_sincos_1 (double a, double da, double x, int4 n, int4 k)
>  	{
>  	  res = do_sin (a, da, &cor);
>  	  cor = 1.035 * cor + __copysign (eps, cor);
> -	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
> +	  retval = ((res == res + cor) ? __copysign (res, a)

OK.

>  		    : sloww1 (a, da, x, k));
>  	}
>        break;
> @@ -418,7 +418,7 @@ do_sincos_2 (double a, double da, double x, int4 n, int4 k)
>  	{
>  	  res = do_sin (a, da, &cor);
>  	  cor = 1.035 * cor + __copysign (eps, cor);
> -	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
> +	  retval = ((res == res + cor) ? __copysign (res, a)

OK.

>  		    : bsloww1 (a, da, x, n));
>  	}
>        break;
> @@ -479,7 +479,7 @@ __sin (double x)
>      {
>        res = do_sin (x, 0, &cor);
>        retval = (res == res + 1.096 * cor) ? res : slow1 (x);
> -      retval = m > 0 ? retval : -retval;
> +      retval = __copysign (retval, x);

OK.

>      }				/*   else  if (k < 0x3feb6000)    */
>  
>  /*----------------------- 0.855469  <|x|<2.426265  ----------------------*/
> @@ -489,7 +489,7 @@ __sin (double x)
>        t = hp0 - fabs (x);
>        res = do_cos (t, hp1, &cor);
>        retval = (res == res + 1.020 * cor) ? res : slow2 (x);
> -      retval = m > 0 ? retval : -retval;
> +      retval = __copysign (retval, x);

OK.

>      }				/*   else  if (k < 0x400368fd)    */
>  
>  #ifndef IN_SINCOS
> @@ -580,7 +580,7 @@ __cos (double x)
>  	{
>  	  res = do_sin (a, da, &cor);
>  	  cor = 1.035 * cor + __copysign (1.0e-31, cor);
> -	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
> +	  retval = ((res == res + cor) ? __copysign (res, a)

OK.

>  		    : sloww1 (a, da, x, 1));
>  	}
>  
> @@ -634,9 +634,9 @@ slow (double x)
>  
>    __dubsin (fabs (x), 0, w);
>    if (w[0] == w[0] + 1.000000001 * w[1])
> -    return (x > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], x);

OK.

>  
> -  return (x > 0) ? __mpsin (x, 0, false) : -__mpsin (-x, 0, false);
> +  return __copysign (__mpsin (fabs (x), 0, false), x);

OK.

>  }
>  
>  /*******************************************************************************/
> @@ -717,7 +717,7 @@ sloww (double x, double dx, double orig, int k)
>    cor = 1.000000001 * w[1] + __copysign (eps, w[1]);
>  
>    if (w[0] == w[0] + cor)
> -    return (x > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], x);

OK.

>  
>    t = (orig * hpinv + toint);
>    xn = t - toint;
> @@ -743,7 +743,7 @@ sloww (double x, double dx, double orig, int k)
>    cor = 1.000000001 * w[1] + __copysign (eps, w[1]);
>  
>    if (w[0] == w[0] + cor)
> -    return (a > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], a);

OK.

>  
>    return k ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
>  }
> @@ -764,7 +764,7 @@ sloww1 (double x, double dx, double orig, int k)
>    res = do_sin_slow (x, dx, 3.1e-30 * fabs (orig), &cor);
>  
>    if (res == res + cor)
> -    return (x > 0) ? res : -res;
> +    return __copysign (res, x);

OK.

>  
>    dx = (x > 0 ? dx : -dx);
>    __dubsin (fabs (x), dx, w);
> @@ -773,7 +773,7 @@ sloww1 (double x, double dx, double orig, int k)
>    cor = 1.000000005 * w[1] + __copysign (eps, w[1]);
>  
>    if (w[0] == w[0] + cor)
> -    return (x > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], x);

OK.

>  
>    return (k == 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
>  }
> @@ -833,7 +833,7 @@ bsloww (double x, double dx, double orig, int n)
>    cor = 1.000000001 * w[1] + __copysign (1.1e-24, w[1]);
>  
>    if (w[0] == w[0] + cor)
> -    return (x > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], x);

OK.

>  
>    return (n & 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
>  }
> @@ -861,7 +861,7 @@ bsloww1 (double x, double dx, double orig, int n)
>    cor = 1.000000005 * w[1] + __copysign (1.1e-24, w[1]);
>  
>    if (w[0] == w[0] + cor)
> -    return (x > 0) ? w[0] : -w[0];
> +    return __copysign (w[0], x);

OK.

>  
>    return (n & 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
>  }
>
  
Siddhesh Poyarekar Sept. 28, 2016, 9:32 p.m. UTC | #4
On Wednesday 28 September 2016 05:59 AM, Manfred wrote:
> The ternary uses m, __copysign uses x. It could be correct (I didn't
> check the full context), but just in case...

m essentially tracks the sign bit of x, so both checks are equivalent.

Siddhesh
  

Patch

diff --git a/sysdeps/ieee754/dbl-64/s_sin.c b/sysdeps/ieee754/dbl-64/s_sin.c
index e4333a4..40d538d 100644
--- a/sysdeps/ieee754/dbl-64/s_sin.c
+++ b/sysdeps/ieee754/dbl-64/s_sin.c
@@ -344,7 +344,7 @@  do_sincos_1 (double a, double da, double x, int4 n, int4 k)
 	{
 	  res = do_sin (a, da, &cor);
 	  cor = 1.035 * cor + __copysign (eps, cor);
-	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
+	  retval = ((res == res + cor) ? __copysign (res, a)
 		    : sloww1 (a, da, x, k));
 	}
       break;
@@ -418,7 +418,7 @@  do_sincos_2 (double a, double da, double x, int4 n, int4 k)
 	{
 	  res = do_sin (a, da, &cor);
 	  cor = 1.035 * cor + __copysign (eps, cor);
-	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
+	  retval = ((res == res + cor) ? __copysign (res, a)
 		    : bsloww1 (a, da, x, n));
 	}
       break;
@@ -479,7 +479,7 @@  __sin (double x)
     {
       res = do_sin (x, 0, &cor);
       retval = (res == res + 1.096 * cor) ? res : slow1 (x);
-      retval = m > 0 ? retval : -retval;
+      retval = __copysign (retval, x);
     }				/*   else  if (k < 0x3feb6000)    */
 
 /*----------------------- 0.855469  <|x|<2.426265  ----------------------*/
@@ -489,7 +489,7 @@  __sin (double x)
       t = hp0 - fabs (x);
       res = do_cos (t, hp1, &cor);
       retval = (res == res + 1.020 * cor) ? res : slow2 (x);
-      retval = m > 0 ? retval : -retval;
+      retval = __copysign (retval, x);
     }				/*   else  if (k < 0x400368fd)    */
 
 #ifndef IN_SINCOS
@@ -580,7 +580,7 @@  __cos (double x)
 	{
 	  res = do_sin (a, da, &cor);
 	  cor = 1.035 * cor + __copysign (1.0e-31, cor);
-	  retval = ((res == res + cor) ? ((a > 0) ? res : -res)
+	  retval = ((res == res + cor) ? __copysign (res, a)
 		    : sloww1 (a, da, x, 1));
 	}
 
@@ -634,9 +634,9 @@  slow (double x)
 
   __dubsin (fabs (x), 0, w);
   if (w[0] == w[0] + 1.000000001 * w[1])
-    return (x > 0) ? w[0] : -w[0];
+    return __copysign (w[0], x);
 
-  return (x > 0) ? __mpsin (x, 0, false) : -__mpsin (-x, 0, false);
+  return __copysign (__mpsin (fabs (x), 0, false), x);
 }
 
 /*******************************************************************************/
@@ -717,7 +717,7 @@  sloww (double x, double dx, double orig, int k)
   cor = 1.000000001 * w[1] + __copysign (eps, w[1]);
 
   if (w[0] == w[0] + cor)
-    return (x > 0) ? w[0] : -w[0];
+    return __copysign (w[0], x);
 
   t = (orig * hpinv + toint);
   xn = t - toint;
@@ -743,7 +743,7 @@  sloww (double x, double dx, double orig, int k)
   cor = 1.000000001 * w[1] + __copysign (eps, w[1]);
 
   if (w[0] == w[0] + cor)
-    return (a > 0) ? w[0] : -w[0];
+    return __copysign (w[0], a);
 
   return k ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
 }
@@ -764,7 +764,7 @@  sloww1 (double x, double dx, double orig, int k)
   res = do_sin_slow (x, dx, 3.1e-30 * fabs (orig), &cor);
 
   if (res == res + cor)
-    return (x > 0) ? res : -res;
+    return __copysign (res, x);
 
   dx = (x > 0 ? dx : -dx);
   __dubsin (fabs (x), dx, w);
@@ -773,7 +773,7 @@  sloww1 (double x, double dx, double orig, int k)
   cor = 1.000000005 * w[1] + __copysign (eps, w[1]);
 
   if (w[0] == w[0] + cor)
-    return (x > 0) ? w[0] : -w[0];
+    return __copysign (w[0], x);
 
   return (k == 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
 }
@@ -833,7 +833,7 @@  bsloww (double x, double dx, double orig, int n)
   cor = 1.000000001 * w[1] + __copysign (1.1e-24, w[1]);
 
   if (w[0] == w[0] + cor)
-    return (x > 0) ? w[0] : -w[0];
+    return __copysign (w[0], x);
 
   return (n & 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
 }
@@ -861,7 +861,7 @@  bsloww1 (double x, double dx, double orig, int n)
   cor = 1.000000005 * w[1] + __copysign (1.1e-24, w[1]);
 
   if (w[0] == w[0] + cor)
-    return (x > 0) ? w[0] : -w[0];
+    return __copysign (w[0], x);
 
   return (n & 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
 }