RFR: JDK-8302028: Port fdlibm atan2 to Java [v2]

[Raffaello Giulietti]

On Fri, 17 Feb 2023 04:18:30 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> Working down the porting list, next stop, atan2.
>
> Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Update regression test.
>  - Merge branch 'master' into JDK-8302028
>  - JDK-8302028: Port fdlibm atan2 to Java

src/java.base/share/classes/java/lang/FdLibm.java line 447:

> 445:             ly = __LO(y);
> 446:             if (((ix | ((lx | -lx) >> 31)) > 0x7ff0_0000)||
> 447:                 ((iy |((ly | - ly) >> 31)) > 0x7ff0_0000))    // x or y is NaN

I think that `>> 31` must be replaced by `>>> 31`.

src/java.base/share/classes/java/lang/FdLibm.java line 458:

> 456:                 case 0, 1 ->  y;              // atan(+/-0, +anything)  = +/-0
> 457:                 case 2    ->  Math.PI + tiny; // atan(+0,   -anything)  =  pi
> 458:                 default   -> -Math.PI - tiny; // atan(-0,   -anything)  = -pi

The original switch statement and this switch expression are semantically equivalent only because of our knowledge that `m` can only assume values 0, 1, 2, or 3. This requires more reasoning than a more verbatim copy of the original statement. Not sure if it is worth.

The same holds for the switch expressions below.

test/jdk/java/lang/Math/Atan2Tests.java line 214:

> 212:             /*
> 213:              * If both arguments are negative infinity, then the result is the
> 214:              * double value closest to -3*pi/4.

Perhaps add a note explaining that high precision computation shows that `-3*PI/4.0` is indeed the `double` closest to -3*pi/4.
Since `PI` is an approximation in the first place, and since there are other two operations (in particular, the multiplication by 3), the claim is not evident.

test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 451:

> 449:             ly = __LO(y);
> 450:             if(((ix|((lx|-lx)>>31))>0x7ff00000)||
> 451:                ((iy|((ly|-ly)>>31))>0x7ff00000))    /* x or y is NaN */

I think that `>>31` must be replaced by `>>>31`.

test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 480:

> 478:                     switch(m) {
> 479:                     case 0: return  zero  ;     /* atan(+...,+INF) */
> 480:                     case 1: return -1.0*zero  ; /* atan(-...,+INF) */

The file in the [netlib repo](https://netlib.org/fdlibm/e_atan2.c) has `-zero` rather than `-1.0*zero`.

-------------

PR: https://git.openjdk.org/jdk/pull/12608