RFR: 8377223: Port fdlibm atanh to Java [v3]

Anton Artemov aartemov at openjdk.org
Thu Feb 19 13:58:27 UTC 2026


On Thu, 19 Feb 2026 12:43:47 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8377223: Addressed reviewer's comments.
>
> src/java.base/share/classes/java/lang/FdLibm.java line 3623:
> 
>> 3621:      *                   := 0.5 * log1p(2x + 2x * x/(1 - x)), if |x| < 0.5.
>> 3622:      *
>> 3623:      *
> 
> Not sure how useful this is in the context here.

Are you concerned about wasted space or anything else?

> src/java.base/share/classes/java/lang/FdLibm.java line 3638:
> 
>> 3636:             double t;
>> 3637:             int hx,ix;
>> 3638:             /*unsigned*/ int lx;
> 
> Suggestion:
> 
>             int lx;  // unsigned

Done.

> src/java.base/share/classes/java/lang/FdLibm.java line 3642:
> 
>> 3640:             lx = __LO(x);                                        /* low word */
>> 3641:             ix = hx & 0x7fff_ffff;
>> 3642:             if ((ix | ((lx | (-lx)) >> 31)) > 0x3ff0_0000) {     /* |x| > 1 */
> 
> Hmm, since `lx` is unsigned this should really be
> Suggestion:
> 
>             if ((ix | ((lx | (-lx)) >>> 31)) > 0x3ff0_0000) {     /* |x| > 1 */
> 
> The tests didn't reveal this bug, so I suggest adding some test cases for this, if feasible.

Addressed. Added two test cases to handle value larger than 1 in abs value.

> src/java.base/share/classes/java/lang/FdLibm.java line 3654:
> 
>> 3652:             if (ix < 0x3fe0_0000) {                              /* x < 0.5 */
>> 3653:                 t = x + x;
>> 3654:                 t = 0.5 * Log1p.compute(t + t*x/(one - x));
> 
> Suggestion:
> 
>                 t = 0.5 * Log1p.compute(t + t * x / (one - x));

Addressed.

> src/java.base/share/classes/java/lang/FdLibm.java line 3656:
> 
>> 3654:                 t = 0.5 * Log1p.compute(t + t*x/(one - x));
>> 3655:             } else
>> 3656:                 t = 0.5 * Log1p.compute((x + x)/(one - x));
> 
> Suggestion:
> 
>                 t = 0.5 * Log1p.compute((x + x) / (one - x));

Addressed.

> test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 2880:
> 
>> 2878:             lx = __LO(x);       /* low word */
>> 2879:             ix = hx&0x7fffffff;
>> 2880:             if ((ix|((lx|(-lx))>>31))>0x3ff00000) /* |x|>1 */
> 
> As above,
> Suggestion:
> 
>             if ((ix|((lx|(-lx))>>>31))>0x3ff00000) /* |x|>1 */

Addressed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828038536
PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828035514
PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828034651
PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828034096
PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828033519
PR Review Comment: https://git.openjdk.org/jdk/pull/29782#discussion_r2828032140


More information about the core-libs-dev mailing list