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

[Andrew Haley]

On Mon, 1 Jul 2024 16:54:55 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).
>> 
>> 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.ATAN2 | 1024 | thrpt | 10 | 0.021 | ops/ms | 135.088 | 32.449 | 4.163 | 135.721 | 32.579 | 4.166
>> Float128Vector.CBRT | 1024 | thrpt | 10 | 0.004 | ops/ms | 114.547 | 39.517 | 2....
>
> Hamlin Li has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 33 commits:
> 
>  - Merge branch 'master' into sleef-aarch64-integrate-source
>  - merge master
>  - sleef 3.6.1 for riscv
>  - sleef 3.6.1
>  - update header files for arm
>  - add inline header file for riscv64
>  - remove notes about sleef changes
>  - fix performance issue
>  - disable unused-function warnings; add log msg
>  - minor
>  - ... and 23 more: https://git.openjdk.org/jdk/compare/2f4f6cc3...b54fc863

There is something that makes me nervous.
The big slab of preprocessed code in libvectormath/sleefinline_rvvm1.h is problematic.
Firstly, in all open source software the code should be the preferred form:

"The source code must be the preferred form in which a programmer would modify the program. Deliberately obfuscated source code is not allowed. Intermediate forms such as the output of a preprocessor or translator are not allowed."
https://opensource.org/osd

Also, any such intermediate form is a golden example of a vector in which to hide something nasty. No one is going to read that file, and a malicious person with access to the JDK source base, either in our own github repo or in many other places downstream of OpenJDK could hide all manner of thing. In its form in this PR it's no better than checking in a binary.
See https://arstechnica.com/security/2024/04/what-we-know-about-the-xz-utils-backdoor-that-almost-infected-the-world/

I'd look at including the SLEEF source code, along with a script which generates the preprocessed form we use in the JDK build, so that more paranoid JDK builders can regenerate the preprocessed code.

Of course, I cannot be sure that my fellow reviewers will agree, but I think it's the right thing to do.

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

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