RFR: 8375285: Port fdlibm asinh to Java [v7]
Raffaello Giulietti
rgiulietti at openjdk.org
Wed Feb 4 11:14:29 UTC 2026
On Wed, 4 Feb 2026 09:12:13 GMT, Anton Artemov <aartemov at openjdk.org> wrote:
>> Hi, please consider the following changes:
>>
>> This is a port of FDLIBM asinh method.
>>
>> Original C vs transliteration port:
>>
>>
>> $ diff -w fdlib_asinh.c Asinh.translit.java
>> 1c1,3
>> < /* asinh(x)
>> ---
>>> /**
>>> * Return the Inverse Hyperbolic Sine of x
>>> *
>> 2a5
>>> *
>> 10a14,17
>>> private static final class Asinh {
>>> private static final double one = 1.0;
>>> private static final double ln2 = 6.93147180559945286227e-01;
>>> private static final double huge = 1.0e300;
>> 12,29c19
>> < #include "fdlibm.h"
>> <
>> < #ifdef __STDC__
>> < static const double
>> < #else
>> < static double
>> < #endif
>> < one = 1.00000000000000000000e+00, /* 0x3FF00000, 0x00000000 */
>> < ln2 = 6.93147180559945286227e-01, /* 0x3FE62E42, 0xFEFA39EF */
>> < huge= 1.00000000000000000000e+300;
>> <
>> < #ifdef __STDC__
>> < double asinh(double x)
>> < #else
>> < double asinh(x)
>> < double x;
>> < #endif
>> < {
>> ---
>>> static double compute(double x) {
>> 39c29
>> < w = __ieee754_log(fabs(x))+ln2;
>> ---
>>> w = log(Math.abs(x))+ln2;
>> 41,42c31,32
>> < t = fabs(x);
>> < w = __ieee754_log(2.0*t+one/(sqrt(x*x+one)+t));
>> ---
>>> t = Math.abs(x);
>>> w = log(2.0*t+one/(sqrt(x*x+one)+t));
>> 45c35
>> < w =log1p(fabs(x)+t/(one+sqrt(one+t)));
>> ---
>>> w =log1p(Math.abs(x)+t/(one+sqrt(one+t)));
>> 47a38
>>> }
>>
>> Transliteration port vs more idiomatic port:
>>
>>
>> $ diff -w Asinh.translit.java Asinh.fdlibm.java
>> 6,9c6,12
>> < * Based on
>> < * asinh(x) = sign(x) * log [ |x| + sqrt(x*x+1) ]
>> < * we have
>> < * asinh(x) := x if 1+x*x=1,
>> ---
>>> *
>>> * asinh(x) is defined so that asinh(sinh(alpha)) = alpha -INF < alpha < < INF
>>> * and sinh(asinh(x)) = x, -INF < x < INF.
>>> * It can be written as asinh(x) = ln(x + sqrt(x^2 + 1)), -INF < x < INF.
>>> * 1. Replace x by |x| as the function is odd.
>>> * 2.
>>> * asinh(x) := x, if 1+x^2 = 1,
>> 12a16,22
>>> *
>>> *
>>> *
>>> * Special cases:
>>> * only asinh(0)=0 is exact for finite x.
>>> * asinh(NaN) is NaN
>>> * asinh(INF) is INF
>> 14,15c24
>> < private static final class Asinh {
>> < private static final double one = 1.0;
>> ---
>>> static final class Asinh {
>> 24c33,35
>> < if(ix>=0x7ff00000) return x+x; /* x is inf or NaN */
>> ---
>>> if(ix >= 0x7ff00000) {
>>> return x...
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>
> 8375285: Addressed reviewer's comments.
src/java.base/share/classes/java/lang/FdLibm.java line 3514:
> 3512: /**
> 3513: * Return the Inverse Hyperbolic Sine of x
> 3514: *
This is inside a javadoc comment.
If you want structuring into paragraphs, you need to add `<p>` at the beginning of new paragraphs, except the 1st.
Alternatively, you could convert to a block comment. In a block comment you can use `<` rather than `<`, but then all the `&...;` HTML named entities make no much sense.
src/java.base/share/classes/java/lang/FdLibm.java line 3520:
> 3518: * asinh(x) is defined so that asinh(sinh(alpha)) = alpha, -∞ < alpha < ∞
> 3519: * and sinh(asinh(x)) = x, -∞ < x < ∞
> 3520: * It can be written as asinh(x) = ln(|x| + sqrt(x^2 + 1)), -∞ < x < ∞
Suggestion:
* It can be written as asinh(x) = sign(x) * ln(|x| + sqrt(x^2 + 1)), -∞ < x < ∞
src/java.base/share/classes/java/lang/FdLibm.java line 3547:
> 3545: return x + x; // x is inf or NaN
> 3546: }
> 3547: if (ix < 0x3e300000) { // |x| < 2**-28
Suggestion:
if (ix < 0x3e30_0000) { // |x| < 2**-28
src/java.base/share/classes/java/lang/Math.java line 2763:
> 2761: /**
> 2762: * Returns the inverse hyperbolic sine of a {@code double} value.
> 2763: * The inverse hyperbolic sine of <i>x</i> is defined to be a function such that
Suggestion:
* The inverse hyperbolic sine of <i>x</i> is defined to be the function such that
Using "the" emphasises that the function is unique.
src/java.base/share/classes/java/lang/Math.java line 2765:
> 2763: * The inverse hyperbolic sine of <i>x</i> is defined to be a function such that
> 2764: * asinh({@linkplain Math#sinh sinh(<i>x</i>)}) = <i>x</i> for any <i>x</i>.
> 2765: * Note that both range and domain of the exact asinh are unrestricted.
Suggestion:
* Note that both domain and range of the exact asinh are unrestricted.
I think that putting the domain first is more important for usage.
src/java.base/share/classes/java/lang/Math.java line 2773:
> 2771: *
> 2772: * <li>If the argument is infinity, then the result is
> 2773: * negative infinity with the same sign as the argument.
Suggestion:
* infinity with the same sign as the argument.
src/java.base/share/classes/java/lang/Math.java line 2777:
> 2775: * <li>If the argument is NaN, then the result is NaN.
> 2776: *
> 2777: * <p>The computed result must be within 2.5 ulps of the exact result.
This paragraph should come _after_ the bulleted list.
src/java.base/share/classes/java/lang/StrictMath.java line 2194:
> 2192: * @return The inverse hyperbolic sine of {@code x}.
> 2193: * @since 27
> 2194: */
Same comments as for Math.
test/jdk/java/lang/Math/HyperbolicTests.java line 1413:
> 1411: * Therefore,
> 1412: *
> 1413: * 1. For large values of x asinh(x) ~= signum(x)
I don't get this.
signum() is either 1, -1, or 0.
test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 2761:
> 2759:
> 2760: /**
> 2761: * Return the Inverse Hyperbolic Sine of x
I suggest turning this into a block comment, and adjust the indentation by one space in the lines below.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763380563
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763380747
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763380943
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763405124
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763405402
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763405924
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763409861
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763413777
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763430265
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763458993
More information about the core-libs-dev
mailing list