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
I think the key question is whether we're comfortable relying on/pointing at an external repository which may or may not be there tomorrow and/or where tags may change outside of our control. The SLEEF source code looks to be around 7.5MB, give or take. That's not enormous, but it's not exactly small when keeping in mind that if we `#include` it in the jdk repo it's going to be there for every cloned repo in every project/branch and very few will actually care about it. I agree that we'd still have to include the pre-generated header files. Hence my suggestion to consider putting it under our control, but in a separate `openjdk` controlled repository. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2229457499
