8350483: AArch64: turn on signum intrinsics by default on Ampere CPUs

[cnqpzhang]

Set -XX:+UseSignumIntrinsic by default for Ampere CPUs. It is to fix performance problem found on JMH cases vm.compiler.Signum|java.lang.*MathBench.sig[nN]um* where fmov is used to transmit data between GPRs and FPRs with significant time cost.

Verified on Ampere-1A and found the scores (thrpt, ops/s) of java.lang.*MathBench.sig[nN]um* improved 40~50%, while vm.compiler.Signum._1_signumFloatTest and vm.compiler.Signum._3_signumDoubleTest results gained exponential increases. Also passed GHA sanity checks, and Jtreg tier1 on Ampere-1A as function-wise smoke tests.

---

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-8350483: AArch64: turn on signum intrinsics by default on Ampere CPUs (Enhancement - P4)

Reviewers

• Andrew Haley (@theRealAph - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23735

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back qpzhang! 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]

@cnqpzhang 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:

8350483: AArch64: turn on signum intrinsics by default on Ampere CPUs

Reviewed-by: aph

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 21 new commits pushed to the master branch:

• 990d40e: 8350476: Fix typo introduced in JDK-8350147
• 39cb493: 8348106: Catch C++ exception in Java_sun_awt_windows_WTaskbarPeer_setOverlayIcon
• ddb2569: 8280682: Refactor AOT code source validation checks
• 65f79c1: 8347335: ZGC: Use limitless mark stack memory
• e410af0: 8342393: Promote commutative vector IR node sharing
• f755fad: 8349653: Clarify the docs for MemorySegment::reinterpret
• a5c9a4d: 8349032: C2: Parse Predicate refactoring in Loop Unswitching broke fix for JDK-8290850
• 302bed0: 8350499: Minimal build fails with slowdebug builds
• 0795d11: 8350464: The flags to set the native priority for the VMThread and Java threads need a broader range
• 05b4812: 8350041: Skip test/jdk/java/lang/String/nativeEncoding/StringPlatformChars.java on static JDK
• ... and 11 more: https://git.openjdk.org/jdk/compare/16033ea79a01b27a7f5cbb31d64da0e80afc7dc3...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 (@theRealAph) 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]

@cnqpzhang The following label will be automatically applied to this pull request:

• hotspot

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

[mlbridge]

Webrevs

• 00: Full (41a8917f)

[theRealAph]

Looks good. I'm looking at Apple M1 which shows no regression with signum intrinsics, although the reliability of JMH benchmarks once we are in the 1 ns range is not great.
So, this is a benefit on Arm and Ampere, and no worse on Apple. Maybe we should think about removing the UseSignumIntrinsic flag altogether.

[theRealAph]

Maybe we should think about removing the UseSignumIntrinsic flag altogether.

Ah, the flag is also used by other ports. But it doesn't make much sense for us not to use the intrinsic.

[cnqpzhang]

Maybe we should think about removing the UseSignumIntrinsic flag altogether.

Ah, the flag is also used by other ports. But it doesn't make much sense for us not to use the intrinsic.

Thanks for your review @theRealAph.

Yes, agree with you that we should turn on signum intrinsics (-XX:+UseSignumIntrinsic), probably -XX:+UseCopySignIntrinsic too. I had a larger test set on Neoverse-N1 and Ampere CPUs, and compared w/ vs w/o these two flags respectively, no obvious performance regression so far. -XX:+UseSignumIntrinsic can produce obvious benefits on fmov cases, while -XX:+UseCopySignIntrinsic can also bring +15% improvements to *MathBench.signum* tests when disabling the signum intrinsics. In Math.java, signum calls copySign, as such the benefit of copysign intrinsics would be hidden in a manner. Therefore, I think they two could be turned on altogether. A JBS ticket is required to track so.

[cnqpzhang]

/integrate

[openjdk]

@cnqpzhang
Your change (at version 41a8917) is now ready to be sponsored by a Committer.

[theRealAph]

Maybe we should think about removing the UseSignumIntrinsic flag altogether.

Ah, the flag is also used by other ports. But it doesn't make much sense for us not to use the intrinsic.

Thanks for your review @theRealAph.

Yes, agree with you that we should turn on signum intrinsics (-XX:+UseSignumIntrinsic), probably -XX:+UseCopySignIntrinsic too. I had a larger test set on Neoverse-N1 and Ampere CPUs, and compared w/ vs w/o these two flags respectively, no obvious performance regression so far. -XX:+UseSignumIntrinsic can produce obvious benefits on fmov cases, while -XX:+UseCopySignIntrinsic can also bring +15% improvements to *MathBench.signum* tests when disabling the signum intrinsics. In Math.java, signum calls copySign, as such the benefit of copysign intrinsics would be hidden in a manner. Therefore, I think they two could be turned on altogether. A JBS ticket is required to track so.

Here's one: JDK-8350663 Are you interested in picking it up?

[cnqpzhang]

Here's one: JDK-8350663 Are you interested in picking it up?

Sure, I will take it. BTW, could you please help sponsor this commit, thanks.

[theRealAph]

/sponsor

[openjdk]

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

• 037e471: 8350666: cmp-baseline builds fail after JDK-8280682
• c8a521f: 8345213: JVM Prefers /etc/timezone Over /etc/localtime on Debian 12
• 86024eb: 8348426: Generate binary file for -XX:AOTMode=record -XX:AOTConfiguration=file
• 267d69b: 8326447: jpackage creates Windows installers that cannot be signed
• 2efb033: 8350601: Miscellaneous updates to jpackage test lib
• d4fdc79: 8344981: [REDO] JDK-6672644 JComboBox still scrolling if switch to another window and return back
• b78043f: 8320220: Compilation of cyclic hierarchy causes infinite recursion
• a3188e0: 8349991: GraphUtils.java can use String.replace() instead of String.replaceAll()
• d422abc: 8350668: has_extra_module_paths in filemap.cpp may be uninitialized
• 829d7a8: 8339889: Several compiler tests ignore vm flags and not marked as flagless
• ... and 37 more: https://git.openjdk.org/jdk/compare/16033ea79a01b27a7f5cbb31d64da0e80afc7dc3...master

Your commit was automatically rebased without conflicts.

[openjdk]

@theRealAph @cnqpzhang Pushed as commit f529bf7.

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

[cnqpzhang]

/backport jdk17u-dev

[openjdk]

@cnqpzhang Could not automatically backport f529bf71 to openjdk/jdk17u-dev due to conflicts in the following files:

• src/hotspot/cpu/aarch64/vm_version_aarch64.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b backport-cnqpzhang-f529bf71-master

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git f529bf712d8946584999dfc98abea60c22c97167

# Backport the commit
$ git cherry-pick --no-commit f529bf712d8946584999dfc98abea60c22c97167
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport f529bf712d8946584999dfc98abea60c22c97167'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport f529bf712d8946584999dfc98abea60c22c97167.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit f529bf71 from the openjdk/jdk repository.

The commit being backported was authored by Patrick Zhang on 26 Feb 2025 and was reviewed by Andrew Haley.

Thanks!

[cnqpzhang]

/backport jdk24u

[openjdk]

@cnqpzhang the backport was successfully created on the branch backport-cnqpzhang-f529bf71-master in my personal fork of openjdk/jdk24u. To create a pull request with this backport targeting openjdk/jdk24u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit f529bf71 from the openjdk/jdk repository.

The commit being backported was authored by Patrick Zhang on 26 Feb 2025 and was reviewed by Andrew Haley.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk24u:

$ git fetch https://github.com/openjdk-bots/jdk24u.git backport-cnqpzhang-f529bf71-master:backport-cnqpzhang-f529bf71-master
$ git checkout backport-cnqpzhang-f529bf71-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk24u.git backport-cnqpzhang-f529bf71-master

⚠️ @cnqpzhang You are not yet a collaborator in my fork openjdk-bots/jdk24u. An invite will be sent out and you need to accept it before you can proceed.