On Tue, 9 Jul 2024 12:08:50 GMT, Hamlin Li <m...@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

If we want the traceability (which I agree is good) of the SLEEF source code 
but want to avoid having it in the jdk repo itself (adding unnecessary "bloat" 
for everybody), perhaps we can consider having it in a separate repository 
somewhere in/under `openjdk`?

It's not immediately clear to me that we need to have support in the JDK build 
system (configure/make) itself for building/updating the header files, as long 
as there's a simple, documented way of doing so. I like to think the 
`createSleef.sh` script is that, but I recognize that I'm biased because I 
wrote it.

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

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

Reply via email to