RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]

[Andrew Haley]

On Tue, 9 Jul 2024 12:08:50 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review the patch?
>> This pr is based on previous work and discussion in [pr 16234](https://github.com/openjdk/jdk/pull/16234), [pr 18294](https://github.com/openjdk/jdk/pull/18294).
>> * NOTE: This pr depends on https://github.com/openjdk/jdk/pull/19185, which includes a README, a script to generate sleef inline headers and generated sleef inline headers.
>> 
>> Compared with previous prs, the major change in this pr is to integrate the source of sleef (for the steps, please check `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than depends on external sleef things (header or lib) at build or run time.
>> Besides of this change, also modify the previous changes accordingly, e.g. remove some uncessary files or changes especially in make dir of jdk.
>> 
>> Besides of the code changes, one important task is to handle the legal process.
>> 
>> Thanks!
>> 
>> ## Test
>> tests:
>> * test/jdk/jdk/incubator/vector/
>> * test/hotspot/jtreg/compiler/vectorapi/
>> 
>> options:
>> * -XX:UseSVE=1 -XX:+EnableVectorSupport -XX:+UseVectorStubs
>> * -XX:UseSVE=0 -XX:+EnableVectorSupport -XX:+UseVectorStubs
>> * -XX:+EnableVectorSupport -XX:-UseVectorStubs
>> 
>> ## Performance
>> 
>> ### Options
>> * +intrinsic: 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions -XX:+EnableVectorSupport -XX:+UseVectorStubs'
>> * -intrinsic: 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions -XX:+EnableVectorSupport -XX:-UseVectorStubs'
>> 
>> ### Float
>> data
>> <google-sheets-html-origin style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
>> Benchmark | (size) | Mode | Cnt | Error | Units | Score +intrinsic (UseSVE=1) | Score -intrinsic | Improvement(UseSVE=1) | Score +intrinsic (UseSVE=0) | Score -intrinsic | Improvement (UseSVE=0)
>> -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | --
>> Float128Vector.ACOS | 1024 | thrpt | 10 | 0.015 | ops/ms | 245.439 | 101.483 | 2.419 | 245.733 | 102.033 | 2.408
>> Float128Vector.ASIN | 1024 | thrpt | 10 | 0.013 | ops/ms | 296.702 | 103.559 | 2.865 | 296.741 | 103.18 | 2.876
>> Float128Vector.ATAN | 1024 | thrpt | 10 | 0.004 | ops/ms | 196.862 | 49.627 | 3.967 | 195.891 | 49.771 | 3.936
>> Float128Vector.ATAN...
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   skip TANH

> Currently,
> 
>     * in [8329816: Add SLEEF version 3.6.1 #19185](https://github.com/openjdk/jdk/pull/19185) it generates the sleef inline headers from sleef 3.6.1, which is tagged in sleef repo.
> 
>     * And with the script in [8329816: Add SLEEF version 3.6.1 #19185](https://github.com/openjdk/jdk/pull/19185), anyone with access to sleef repo can re-generate these inline headers by himself

Right, but think about package builders. This isn't about J Random Hacker doing it by hand.

When a package gets built, the builder machine unpacks source code. If SLEEF is included as part of JDK source, all the builder has to do is run the script and overwrite whatever preprocessed source is in there. The alternative is packaging the SLEEF source code tarball separately in the OpenJDK source package. Sure, all of this can be done, but it's a question of whether we do it once, here, now, or all the downstream builders have to do it themselves.

> ( in fact anyone can generate the inline headers from sleef from scratch without using scripts in [8329816: Add SLEEF version 3.6.1 #19185](https://github.com/openjdk/jdk/pull/19185), our script just make it easy for the future maintenance), so it's easy for anyone to verify these inline header files used in jdk.

That script must be checked in to the OpenJDK tree.

> With these 2 points, seems the traceability is fine to me, please kindly point out if I missed some points. Maybe we can add some more clear and specific information in README or createSleef.sh in #19185 to indicate which version of sleef source we're using in jdk.
> 
> I'm also fine with your suggestion to add whole sleef repo into jdk (maybe we can remove some of files, but we can ignore the difference temporarily in the dicussion here). To copy the sleef repo into jdk, we still need to pre-generate the inline header files, and check them in jdk along with the sleef repo, I think you also think so too

Yes.

> (As without checking in these inline headers, we will have to bring some extra dependencies into jdk, and increase extra compilation time when building jdk). But from traceability point of view, seems to me it does not bring extra benefit than current #19185. For example, if someone want to verify the pre-generate inline headers in jdk, he need to first verify the sleef source in jdk, then the pre-generated sleef inline headers.

You don't need to verify the pre-generated inline headers, just overwrite them. The point is that the sleef source is digitally signed, not just by the SLEEF maintainers, _but by OpenJDK as well._ This is not a small thing.

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

PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2230332083