8360559: Optimize Math.sinh for x86 64 bit platforms #26152

missa-prime · 2025-07-07T03:05:15Z

The goal of this PR is to implement an x86_64 intrinsic for java.lang.Math.sinh() using libm. There is a new set of micro-benchmarks are included to check the performance of specific input value ranges to help prevent regressions in the future.

The command to run all range specific micro-benchmarks is posted below.

make test TEST="micro:SinhPerf.SinhPerfRanges"

The results of all tests posted below were captured with an Intel® Xeon 8488C using OpenJDK v26-b4 as the baseline version.

For performance data collected with the new built in range micro-benchmark, see the table below. Each result is the mean of 8 individual runs, and the input ranges used match those from the original Java implementation. Overall, the intrinsic provides an an average uplift of 64% when input values fall into the middle three ranges where heavy computation is required. However, very small inputs and very large inputs show drops of 74% and 66% respectively.

Input range(s)	Baseline throughput (ops/ms)	Intrinsic throughput (ops/ms)	Speedup
[-2^(-28), 2^(-28)]	844160	216029	0.26x
[-22, -2^(-28)], [2^(-28), 22]	81662	157351	1.93x
[-709.78, -22], [22, 709.78]	119075	167635	1.41x
[-710.48, -709.78], [709.78, 710.48]	111636	177125	1.59x
(-INF, -710.48], [710.48, INF)	959296	313839	0.33x

Finally, the jtreg:test/jdk/java/lang/Math/HyperbolicTests.java test passed with the changes.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8360559: Optimize Math.sinh for x86 64 bit platforms (Enhancement - P4)

Reviewers

Srinivas Vamsi Parasa (@vamsi-parasa - Author) Review applies to c0adc8b3
Sandhya Viswanathan (@sviswa7 - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26152/head:pull/26152
$ git checkout pull/26152

Update a local copy of the PR:
$ git checkout pull/26152
$ git pull https://git.openjdk.org/jdk.git pull/26152/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26152

View PR using the GUI difftool:
$ git pr show -t 26152

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26152.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-07-07T03:05:57Z

👋 Welcome back missa! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-07-07T03:06:08Z

@missa-prime This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8360559: Optimize Math.sinh for x86 64 bit platforms

Reviewed-by: sviswanathan, sparasa

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 467 new commits pushed to the master branch:

b5f450a: 8364226: Better ECDSASignature Memory Management
6c52b73: 8209992: Align SSLSocket and SSLEngine Javadocs
567c0c9: 8354244: Use random data in MinMaxRed_Long data arrays
... and 464 more: https://git.openjdk.org/jdk/compare/1ca008fd02496dc33e2707c102560cae1690fba5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@sviswa7) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2025-07-07T03:06:47Z

@missa-prime The following labels will be automatically applied to this pull request:

core-libs
graal
hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-07-07T03:10:14Z

Webrevs

fweimer-rh · 2025-07-07T04:48:18Z

Isn't this effectively a libm benchmark (where the sinh implementation resides)? Please include the libm version for reference here. Thanks.

jddarcy · 2025-07-07T18:01:26Z

Are there plans for a cosh intrinsic as follow-up work? There was a previous effort to intrinsify tanh.

jddarcy · 2025-07-07T18:03:54Z

A large fraction of floating-point numbers are in the [-2^(-28), 2^(-28)] range where there is a performance degradation with the new implementation.

What is the expected distribution of arguments to sinh?

missa-prime · 2025-07-07T23:33:45Z

A large fraction of floating-point numbers are in the [-2^(-28), 2^(-28)] range where there is a performance degradation with the new implementation.

What is the expected distribution of arguments to sinh?

I don't know about an expected distribution of distribution of arguments to sinh. From glancing through research papers though, I get the sense that the [-2^(-28), 2^(-28)] range isn't used all that much. A couple of papers showing this are linked below.

I think the [-2^(-28), 2^(-28)] range isn't all that interesting for sinh anyway as it just returns the input itself. So, I doubt any applications are relying all that heavily on it.

missa-prime · 2025-07-07T23:45:08Z

Are there plans for a cosh intrinsic as follow-up work? There was a previous effort to intrinsify tanh.

Yes, the intention is to have cosh as next math intrinsic.

missa-prime · 2025-07-07T23:53:14Z

Isn't this effectively a libm benchmark (where the sinh implementation resides)? Please include the libm version for reference here. Thanks.

I'm not sure which libm version you're referring to. The only current sinh benchmark I'm aware of is in MathBench.java, and the benchmarks in SinhPerf.java didn't exist until now. Could you clarify?

jddarcy · 2025-07-10T04:52:01Z

What is the accuracy of the algorithm in ulps (units the last place)?

The java.lang.Math specification has max error and various other accuracy requirements any implementation must meet.

fweimer-rh · 2025-07-10T10:04:36Z

I'm not sure which libm version you're referring to. The only current sinh benchmark I'm aware of is in MathBench.java, and the benchmarks in SinhPerf.java didn't exist until now. Could you clarify?

I didn't notice the code you ported over from the Intel math library at first. I assumed “libm” referred to the system libm version.

missa-prime · 2025-07-10T14:31:02Z

What is the accuracy of the algorithm in ulps (units the last place)?

The java.lang.Math specification has max error and various other accuracy requirements any implementation must meet.

0.51 ulps

jddarcy · 2025-07-10T21:23:50Z

What is the accuracy of the algorithm in ulps (units the last place)?
The java.lang.Math specification has max error and various other accuracy requirements any implementation must meet.

0.51 ulps

Please add that as a comment to the changeset.

Also, if the worst-case inputs for this algorithm are known, they should the added as part of the core libraries regression tests for the math library.

missa-prime · 2025-07-17T18:49:00Z

What is the accuracy of the algorithm in ulps (units the last place)?
The java.lang.Math specification has max error and various other accuracy requirements any implementation must meet.

0.51 ulps

Please add that as a comment to the changeset.

Also, if the worst-case inputs for this algorithm are known, they should the added as part of the core libraries regression tests for the math library.

I included the error bound as a separate section of the comment header in the stub generator source file. Also, the input ranges are covered in jtreg:test/jdk/java/lang/Math/HyperbolicTests.java.

vamsi-parasa

I reviewed the code, ran the benchmarks and correctness test and verified that it's passing. This PR looks good to me.

src/hotspot/cpu/x86/stubGenerator_x86_64_sinh.cpp

…_0_2

sviswa7

Looks good to me.

missa-prime · 2025-08-01T02:44:16Z

@eme64 Available to run the additional tests? Also, a couple of the pre-submit checks had failures unrelated to the code changes.

missa-prime · 2025-08-04T17:50:04Z

/integrate

openjdk · 2025-08-04T17:51:35Z

@missa-prime
Your change (at version 3ab7ab3) is now ready to be sponsored by a Committer.

sviswa7 · 2025-08-04T18:23:51Z

/sponsor

openjdk · 2025-08-04T18:48:03Z

Going to push as commit 05f8a6f.
Since your change was applied there have been 467 commits pushed to the master branch:

b5f450a: 8364226: Better ECDSASignature Memory Management
6c52b73: 8209992: Align SSLSocket and SSLEngine Javadocs
567c0c9: 8354244: Use random data in MinMaxRed_Long data arrays
... and 464 more: https://git.openjdk.org/jdk/compare/1ca008fd02496dc33e2707c102560cae1690fba5...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-08-04T18:48:58Z

@sviswa7 @missa-prime Pushed as commit 05f8a6f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

RogerRiggs · 2025-08-04T19:06:10Z

This seems to be failing in the CI on the build of: linux-x64-debug-nopch

[2025-08-04T19:02:29,373Z] /......../workspace/open/src/hotspot/cpu/x86/stubGenerator_x86_64_sinh.cpp:293:27: error: 'stub_id' was not declared in this scope; did you mean 'StubId'?
[2025-08-04T19:02:29,373Z]   293 |   StubCodeMark mark(this, stub_id);
[2025-08-04T19:02:29,373Z]       |                           ^~~~~~~
[2025-08-04T19:02:29,373Z]       |                           StubId
[2025-08-04T19:02:29,373Z]

missa-prime · 2025-08-04T19:47:32Z

This seems to be failing in the CI on the build of: linux-x64-debug-nopch

[2025-08-04T19:02:29,373Z] /......../workspace/open/src/hotspot/cpu/x86/stubGenerator_x86_64_sinh.cpp:293:27: error: 'stub_id' was not declared in this scope; did you mean 'StubId'?
[2025-08-04T19:02:29,373Z]   293 |   StubCodeMark mark(this, stub_id);
[2025-08-04T19:02:29,373Z]       |                           ^~~~~~~
[2025-08-04T19:02:29,373Z]       |                           StubId
[2025-08-04T19:02:29,373Z]

Thanks for notification. It looks like an incorrect type declaration. I'll submit a fix.

eme64 · 2025-08-19T08:12:10Z

@missa-prime I was on vacation for a few weeks. Feel free to ping others when I don't respond ;)

x86_64 intrinsic for sinh using libm

9c5cc60

openjdk bot added the rfr Pull request is ready for review label Jul 7, 2025

openjdk bot added graal [email protected] hotspot [email protected] core-libs [email protected] labels Jul 7, 2025

Move error bound to separate section in comment header

c0adc8b

vamsi-parasa approved these changes Jul 28, 2025

View reviewed changes

sviswa7 reviewed Jul 30, 2025

View reviewed changes

src/hotspot/cpu/x86/stubGenerator_x86_64_sinh.cpp Outdated Show resolved Hide resolved

Make sure xmm1 is initialized before potential use in L_2TAG_PACKET_3…

3ab7ab3

…_0_2

sviswa7 approved these changes Jul 31, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 31, 2025

openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 4, 2025

openjdk bot added the integrated Pull request has been integrated label Aug 4, 2025

openjdk bot closed this Aug 4, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Aug 4, 2025

missa-prime deleted the user/missa-prime/sinh branch August 4, 2025 20:21

dcubed-ojdk mentioned this pull request Aug 4, 2025

8364666: Tier1 builds broken by JDK-8360559 #26629

Closed

3 tasks

8360559: Optimize Math.sinh for x86 64 bit platforms #26152

8360559: Optimize Math.sinh for x86 64 bit platforms #26152

Conversation

missa-prime commented Jul 7, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 7, 2025

Uh oh!

openjdk bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jul 7, 2025

Uh oh!

mlbridge bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

fweimer-rh commented Jul 7, 2025

Uh oh!

jddarcy commented Jul 7, 2025

Uh oh!

jddarcy commented Jul 7, 2025

Uh oh!

missa-prime commented Jul 7, 2025

Uh oh!

missa-prime commented Jul 7, 2025

Uh oh!

missa-prime commented Jul 7, 2025

Uh oh!

jddarcy commented Jul 10, 2025

Uh oh!

fweimer-rh commented Jul 10, 2025

Uh oh!

missa-prime commented Jul 10, 2025

Uh oh!

jddarcy commented Jul 10, 2025

Uh oh!

missa-prime commented Jul 17, 2025

Uh oh!

vamsi-parasa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sviswa7 left a comment

Choose a reason for hiding this comment

Uh oh!

missa-prime commented Aug 1, 2025

Uh oh!

missa-prime commented Aug 4, 2025

Uh oh!

openjdk bot commented Aug 4, 2025

Uh oh!

sviswa7 commented Aug 4, 2025

Uh oh!

openjdk bot commented Aug 4, 2025

Uh oh!

openjdk bot commented Aug 4, 2025

Uh oh!

RogerRiggs commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

missa-prime commented Aug 4, 2025

Uh oh!

eme64 commented Aug 19, 2025

Uh oh!

Uh oh!

missa-prime commented Jul 7, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jul 7, 2025 •

edited

Loading

mlbridge bot commented Jul 7, 2025 •

edited

Loading

RogerRiggs commented Aug 4, 2025 •

edited

Loading