Re: signed overflow in atan2

2018-02-14 Thread Tom Cosgrove
>>> Eitan Adler 14-Feb-18 08:09 >>>
>
> Hi all,
>
> you may want the following patch. Previous discussion:
> https://lists.freebsd.org/pipermail/freebsd-numerics/2018-February/thread.html
>
> Original submission: https://github.com/freebsd/freebsd/pull/130
>
>
> ===
>
> As a component of atan2(y, x), the case of x == 1.0 is farmed out to
> atan(y). The current implementation of this comparison is vulnerable
> to signed integer underflow (that is, undefined behavior), and it's
> performed in a somewhat more complicated way than it need be. Change
> it to not be quite so cute, rather directly comparing the high/low
> bits of x to the specific IEEE-754 bit pattern that encodes 1.0.
>
> Note that while there are three different e_atan* files in the
> relevant directory, only this one needs fixing. e_atan2f.c already
> compares against the full bit pattern encoding 1.0f, while
> e_atan2l.cuses bitwise-ands/ors/nots and so doesn't require a change.
> ===
>
>
>
> Index: e_atan2.c
> ===
> RCS file: /cvs/src/lib/libm/src/e_atan2.c,v
> retrieving revision 1.13
> diff -u -r1.13 e_atan2.c
> --- e_atan2.c 12 Sep 2016 19:47:02 - 1.13
> +++ e_atan2.c 14 Feb 2018 08:06:05 -
> @@ -64,7 +64,7 @@
>   if(((ix|((lx|-lx)>>31))>0x7ff0)||
>  ((iy|((ly|-ly)>>31))>0x7ff0)) /* x or y is NaN */
>  return x+y;
> - if(((hx-0x3ff0)|lx)==0) return atan(y);   /* x=1.0 */
> + if(hx==0x3ff0&==0) return atan(y);   /* x=1.0 */

Wouldn't it work just as well, with fewer jumps, and match the original
intent, to replace the subtraction with xor?

 if(((hx^0x3ff0)|lx)==0) return atan(y);   /* x=1.0 */

>   m = ((hy>>31)&1)|((hx>>30)&2); /* 2*sign(x)+sign(y) */
>
>  /* when y = 0 */
>
>

Tom



signed overflow in atan2

2018-02-14 Thread Eitan Adler
Hi all,

you may want the following patch. Previous discussion:
https://lists.freebsd.org/pipermail/freebsd-numerics/2018-February/thread.html

Original submission: https://github.com/freebsd/freebsd/pull/130


===

As a component of atan2(y, x), the case of x == 1.0 is farmed out to
atan(y). The current implementation of this comparison is vulnerable
to signed integer underflow (that is, undefined behavior), and it's
performed in a somewhat more complicated way than it need be. Change
it to not be quite so cute, rather directly comparing the high/low
bits of x to the specific IEEE-754 bit pattern that encodes 1.0.

Note that while there are three different e_atan* files in the
relevant directory, only this one needs fixing. e_atan2f.c already
compares against the full bit pattern encoding 1.0f, while
e_atan2l.cuses bitwise-ands/ors/nots and so doesn't require a change.
===



Index: e_atan2.c
===
RCS file: /cvs/src/lib/libm/src/e_atan2.c,v
retrieving revision 1.13
diff -u -r1.13 e_atan2.c
--- e_atan2.c 12 Sep 2016 19:47:02 - 1.13
+++ e_atan2.c 14 Feb 2018 08:06:05 -
@@ -64,7 +64,7 @@
  if(((ix|((lx|-lx)>>31))>0x7ff0)||
 ((iy|((ly|-ly)>>31))>0x7ff0)) /* x or y is NaN */
 return x+y;
- if(((hx-0x3ff0)|lx)==0) return atan(y);   /* x=1.0 */
+ if(hx==0x3ff0&==0) return atan(y);   /* x=1.0 */
  m = ((hy>>31)&1)|((hx>>30)&2); /* 2*sign(x)+sign(y) */

 /* when y = 0 */


-- 
Eitan Adler