jdk
jdk copied to clipboard
openjdk
8290249: Vectorize signum on AArch64
This patch auto-vectorizes Math.signum intrinsic for float and double types on aarch64 (Neon and SVE). On SVE supporting machines, if the MaxVectorSize <=16 the Neon code would be emitted and if the MaxVectorSize > 16, the SVE code for the intrinsic would be emitted.
Following is the performance data for the micro test here - test/micro/org/openjdk/bench/vm/compiler/VectorSignum.java
Benchmark Size A B C
VectorSignum.doubleSignum 256 1.79 1.70 3.18
VectorSignum.doubleSignum 512 1.86 1.73 3.69
VectorSignum.doubleSignum 1024 1.89 1.74 2.98
VectorSignum.doubleSignum 2048 1.92 1.75 3.04
VectorSignum.floatSignum 256 3.34 3.06 3.92
VectorSignum.floatSignum 512 3.63 3.22 5.27
VectorSignum.floatSignum 1024 3.76 3.35 4.77
VectorSignum.floatSignum 2048 3.85 3.47 5.59
A, B , C machine descriptions given below - A : 128-bit Neon machine B : 256-bit SVE machine C : 512-bit SVE machine
The numbers in the table are the gain ratios between the runtime (ns/op) of the scalar, non-vectorized intrinsic code and the vectorized version of the intrinsic (this patch).
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8290249: Vectorize signum on AArch64
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9807/head:pull/9807
$ git checkout pull/9807
Update a local copy of the PR:
$ git checkout pull/9807
$ git pull https://git.openjdk.org/jdk pull/9807/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 9807
View PR using the GUI difftool:
$ git pr show -t 9807
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9807.diff
:wave: Welcome back Bhavana-Kilambi! 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.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@Bhavana-Kilambi The following label will be automatically applied to this pull request:
hotspot-compiler
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.
Webrevs
Hello, thank you for reviewing my patch. I have made the changes as suggested and waiting for the refactoring patch to be merged. I will then change my *ad files accordingly and put another patch for review in this PR.
Hello, thank you for reviewing my patch. I have made the changes as suggested and waiting for the refactoring patch to be merged. I will then change my *ad files accordingly and put another patch for review in this PR.
The change to assembler.hpp is still not done.
Hello, thank you for reviewing my patch. I have made the changes as suggested and waiting for the refactoring patch to be merged. I will then change my *ad files accordingly and put another patch for review in this PR.
The change to assembler.hpp is still not done. I mean I will put up the entire patch (with changes to the assember_aarch64.hpp, c2_MacroAssembler_aarch64.cpp as suggested by you and also the *ad files) once the refactoring patch is merged. I will need to make changes to the *ad files once the refactoring patch is merged, so I plan to put them all together in a single patch. Apologies if I wasn't clear.
Hi, I just pushed a new commit with the proposed changes (and a few others). Please review. Once the refactoring patch is merged, I will rebase/merge this patch accordingly. Thank you.
The builds on aarch64 have failed as I missed adding parantheses in the assembler.hpp file. Will update with the new patch shortly.
@Bhavana-Kilambi 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:
8290249: Vectorize signum on AArch64
Reviewed-by: aph, ngasson
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 19 new commits pushed to the master branch:
- 27b0f7726b70127c0ed714cfc1041d3da71a9dc3: 8292318: Memory corruption in remove_dumptime_info
- 9a65524e2f98c1b4e253dcb637a708cec7b591bc: 8290300: Use standard String-joining tools where applicable
- f9004fe4438c30eb639e3c36e6531c306b836e36: 8292561: Make "ReplayCompiles" a diagnostic product switch
- 2fbb9362032df26582c389b7114cf0a215ed3afd: 8292691: Move CompilerConfig::is_xxx() inline functions out of compilerDefinitions.hpp
- 3601e30df794db122d8d04fb3c04868ccbaa0baf: 8290909: MemoryPoolMBean/isUsageThresholdExceeded tests failed with "isUsageThresholdExceeded() returned false, and is still false, while threshold = MMMMMMM and used peak = NNNNNNN"
- 37c0a13647e74fd02823a3f621e986f96904b933: 8292350: Use static methods for hashCode/toString primitives
- 44532009fff11884aa6f16a997b771c41cb01d2f: 8292628: x86: Improve handling of constants in trigonometric stubs
- 07c9ba74fa3baebffcc15d3ee6ef941edf6be1a3: 8292686: runtime/cds/appcds/TestWithProfiler.java SIGSEGV in TableStatistics ctr
- 235151ead89f9102e3a57ba8f88807f180887866: 8292676: Remove two kerberos tests from problem list
- df5209e70fd92ec6bda4e7356a3ad121732f6c66: 8292683: Remove BadKeyUsageTest.java from Problem List
- ... and 9 more: https://git.openjdk.org/jdk/compare/f2f0cd86bf4dce4633f484476077fd090549780e...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, @nick-arm) 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).
@Bhavana-Kilambi Your change (at version 221405e0b2320f56515a5e4aa613f41333dffa3d) is now ready to be sponsored by a Committer.
Going to push as commit 07c797720d68d5a36d438f2e9da8b72f247f22fc.
Since your change was applied there have been 20 commits pushed to the master branch:
- a3ec0bb03a5de805fc4b45477925aa18b875bc79: 8253413: [REDO] [REDO] G1 incorrectly limiting young gen size when using the reserve can result in repeated full gcs
- 27b0f7726b70127c0ed714cfc1041d3da71a9dc3: 8292318: Memory corruption in remove_dumptime_info
- 9a65524e2f98c1b4e253dcb637a708cec7b591bc: 8290300: Use standard String-joining tools where applicable
- f9004fe4438c30eb639e3c36e6531c306b836e36: 8292561: Make "ReplayCompiles" a diagnostic product switch
- 2fbb9362032df26582c389b7114cf0a215ed3afd: 8292691: Move CompilerConfig::is_xxx() inline functions out of compilerDefinitions.hpp
- 3601e30df794db122d8d04fb3c04868ccbaa0baf: 8290909: MemoryPoolMBean/isUsageThresholdExceeded tests failed with "isUsageThresholdExceeded() returned false, and is still false, while threshold = MMMMMMM and used peak = NNNNNNN"
- 37c0a13647e74fd02823a3f621e986f96904b933: 8292350: Use static methods for hashCode/toString primitives
- 44532009fff11884aa6f16a997b771c41cb01d2f: 8292628: x86: Improve handling of constants in trigonometric stubs
- 07c9ba74fa3baebffcc15d3ee6ef941edf6be1a3: 8292686: runtime/cds/appcds/TestWithProfiler.java SIGSEGV in TableStatistics ctr
- 235151ead89f9102e3a57ba8f88807f180887866: 8292676: Remove two kerberos tests from problem list
- ... and 10 more: https://git.openjdk.org/jdk/compare/f2f0cd86bf4dce4633f484476077fd090549780e...master
Your commit was automatically rebased without conflicts.
@nick-arm @Bhavana-Kilambi Pushed as commit 07c797720d68d5a36d438f2e9da8b72f247f22fc.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.