Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
On Tue, 16 Jul 2024 10:42:24 GMT, Andrew Haley wrote: > We're only a couple of weeks away from the summit. What would be a long time? OK, then let's wait for it. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2230591233
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Mon, 15 Jul 2024 17:35:59 GMT, Ludovic Henry wrote: >>> > I can't tell what problem we're trying to solve by not simply checking in >>> > the source code, in its preferred form, to the OpenJDK tree. Thhis has >>> > practical advantages to do with traceability and security, and >>> > in-principle reasons to do with basic Open Source practice too. On the >>> > other side, there are no disadvantages. >>> >>> Do you suggest to copy the whole sleef source repo into jdk? >> >> I think so, along with scripting that generates the preprocessed file we >> use. It might be the case that there are some sleef files not used at all >> they could be omitted, but I'm not sure it would be useful, and from a >> traceability point of view it's probably best to grab it all, unless it's >> really huge > >> > > I can't tell what problem we're trying to solve by not simply checking >> > > in the source code, in its preferred form, to the OpenJDK tree. Thhis >> > > has practical advantages to do with traceability and security, and >> > > in-principle reasons to do with basic Open Source practice too. On the >> > > other side, there are no disadvantages. >> > >> > >> > Do you suggest to copy the whole sleef source repo into jdk? >> >> I think so, along with scripting that generates the preprocessed file we >> use. It might be the case that there are some sleef files not used at all >> they could be omitted, but I'm not sure it would be useful, and from a >> traceability point of view it's probably best to grab it all, unless it's >> really huge > > Given the Sleef build system currently uses cmake, we would have two choices > to build the header files as part of the OpenJDK build system: > 1. take a dependency on cmake in order to build the Sleef headers > 2. write a custom build system for Sleef to integrate into OpenJDK > > Neither approach sound good to me as a mandatory option. > > However, if we are to allow the person building OpenJDK to _optionally_ > generate the headers from a Sleef source checkout (provided by the user with > a `--with-sleef-src=/path/to/sleef`), we can then more easily take the > assumption that the user has installed the necessary dependencies. That would > also be in line with how binutils is being built and integrated. > _On a slightly separate note (and I see @luhenry is in this comment thread > too and has contributed to SLEEF) it will be good if this can be used to > enhance the performance on RISC-V too in the future ;-)_ We already had a prototype which depends on this pr, and the performance gain is promising. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2230579500
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
On Tue, 16 Jul 2024 09:48:55 GMT, Andrew Haley wrote: >>> 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 ... > >> @theRealAph Thanks for clarification. >> >> I think there are several different parts involved in the above discussion, >> please kindly correct me if I misunderstood. >> >> 1. package builders. This is about the release of jdk (both src and >> binary), by either openjdk, adoptium, or any other downstream vendors. >> >> 2. jdk daily development. This is about to modify, build, run/test jdk >> daily by jdk developers. >> >> For the package builders, original sleef source is > > may be > >> necessary; for the jdk daily development, only pre-generated sleef inline >> headers are necessary. > > Yes, most of the time. Some devs will want to be more thorough. > >> The script to pre-generate sleef inline headers is only triggerred by >> package builders (and I think it involves some scripts which are not part of >> jdk source ? e.g. the script to trigger pre-generating script), > > No: all of the scripts to generate the preprocessed source from the SLEEF > source must in the OpenJDK source. > >> but for jdk daily development, we just need pre-generated sleef inline >> headers. Am I understanding correctly above? > > Yes, most of the time. > > Bear in mind that convenient daily development of OpenJDK is important, > because we don't want to discourage developers. But we've never treated the > size of the repo as one of our primary considerations. @theRealAph I see, I think now I understand the whole picture of your concerns. Thanks! > 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. Based on @vidmik 's previous comments, I think we all agree original sleef source should be added into jdk,
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
On Tue, 16 Jul 2024 08:35:25 GMT, Andrew Haley wrote: >> 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 di... @theRealAph Thanks for clarification. I think there are several different parts involved in the above discussion, please kindly correct me if I misunderstood. 1. package builders. This is about the release of jdk (both src and binary), by either openjdk, adoptium, or any other downstream vendors. 2. jdk daily development. This is about to modify, build, run/test jdk daily by jdk developers. For the package builders, original sleef source is necessary; for the jdk daily development, only pre-generated sleef inline headers are necessary. The script to pre-generate sleef inline headers is only triggerred by package builders (and I think it involves some scripts which are not part of jdk source ? e.g. the script to trigger pre-generating script), but for jdk daily development, we just need pre-generated sleef inline headers. Am I understanding correctly above? - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2230456463
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Mon, 15 Jul 2024 17:35:59 GMT, Ludovic Henry wrote: > I think so, along with scripting that generates the preprocessed file we use. > It might be the case that there are some sleef files not used at all they > could be omitted, but I'm not sure it would be useful, and from a > traceability point of view it's probably best to grab it all, unless it's > really huge Currently, * in 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 https://github.com/openjdk/jdk/pull/19185, anyone with access to sleef repo can re-generate these inline headers by himself( in fact anyone can generate the inline headers from sleef from scratch without using scripts in 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. 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 https://github.com/openjdk/jdk/pull/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 (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 https://github.com/openjdk/jdk/pull/19185. If someone want to verify the pre-generate inline headers in jdk, he still need to verify the sleef source in jdk, then the pre-generated sleef inline headers. How do you think about it? - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2229421715
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Wed, 10 Jul 2024 10:48:19 GMT, Andrew Haley wrote: > I can't tell what problem we're trying to solve by not simply checking in the > source code, in its preferred form, to the OpenJDK tree. Thhis has practical > advantages to do with traceability and security, and in-principle reasons to > do with basic Open Source practice too. On the other side, there are no > disadvantages. Do you suggest to copy the whole sleef source repo into jdk? - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2228672993
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Mon, 8 Jul 2024 16:20:40 GMT, Andrew Haley wrote: > I finally did some measurements. Thanks for testing it! > It would be nice if the JMH test were part of this patch. OK, I can do that later. > > It mostly looks good, but I can see an odd regression of DoubleMaxVector.TANH > (by 39%) on Apple M1. I don't really know why this is, given that tanh(x) is > almost certainly based on expm1(x). This probably isn't important, but it is > odd. Yes, it has some regression in TANH, I have modified the code to skip TANH (https://github.com/openjdk/jdk/pull/18605/commits/6061c25de00423f2c92c08ce40af4815c0fa3933) - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2220231384
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
> 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 > > 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.72... Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: skip TANH - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/da65cfa5..6061c25d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=10 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=09-10 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v10]
> 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 > > 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.72... Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: minor - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/b54fc863..da65cfa5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=09 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=08-09 Stats: 17 lines in 3 files changed: 11 ins; 4 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v3]
On Thu, 27 Jun 2024 22:03:37 GMT, Mikael Vidstedt wrote: >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to >> optimize vector math operations by leveraging the SLEEF library. For legal >> reasons the actual contribution of the SLEEF files needs to be handled >> separately. This enhancement adds the relevant files, enabling the rest of >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Update README to include RISC-V An quick update, I just verified via QEMU that sleefinline_rvvm1.h works well too. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2214322358
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Mon, 8 Jul 2024 08:43:34 GMT, Andrew Haley wrote: > That doesn't work. > > ``` > Running tests using MICRO control variable > 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions > -XX:+EnableVectorSupport -XX:+UseVectorStubs' > Unknown test selection: > 'org.openjdk.bench.jdk.incubator.vector.operation.Float' > ``` I think by copying the Float*.java and dependent files under test/micro/org/openjdk/bench/jdk/incubator/vector/operation/ from vectorIntrinsics branch in panama-vector repo can resolve the issue. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2213501489
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Fri, 5 Jul 2024 17:44:14 GMT, Andrew Haley wrote: > I also had problems with javac running out of heap space, which was very odd. > I fixed it with this: > > ``` > diff --git a/make/autoconf/boot-jdk.m4 b/make/autoconf/boot-jdk.m4 > index 8d272c28ad5..617ccfd8fff 100644 > --- a/make/autoconf/boot-jdk.m4 > +++ b/make/autoconf/boot-jdk.m4 > @@ -470,7 +470,7 @@ AC_DEFUN_ONCE([BOOTJDK_SETUP_BOOT_JDK_ARGUMENTS], ># Maximum amount of heap memory. >JVM_HEAP_LIMIT_32="768" ># Running a 64 bit JVM allows for and requires a bigger heap > - JVM_HEAP_LIMIT_64="1600" > + JVM_HEAP_LIMIT_64="6400" > ``` For the command to run the tests, I use `make test TEST=org.openjdk.bench.jdk.incubator.vector.operation.Float" MICRO="FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions -XX:UseSVE=1 -XX:+EnableVectorSupport -XX:+UseVectorStubs"`. I just copy and run Double/Float benchmark tests (without copying other tests under `org.openjdk.bench.jdk.incubator.vector.operation`), in which way I think it will not have this OOM issue. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2213280630
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v3]
On Thu, 27 Jun 2024 22:03:37 GMT, Mikael Vidstedt wrote: >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to >> optimize vector math operations by leveraging the SLEEF library. For legal >> reasons the actual contribution of the SLEEF files needs to be handled >> separately. This enhancement adds the relevant files, enabling the rest of >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Update README to include RISC-V Just some minor comments in README, otherwise looks good. src/jdk.incubator.vector/linux/native/libvectormath/README line 18: > 16: NOTE: The following cmake options are necessary when building SLEEF: > 17: * -DSLEEF_BUILD_INLINE_HEADERS=ON > 18: * -DSLEEF_ENFORCE_SVE=ON `-DSLEEF_ENFORCE_SVE=ON` can be removed. src/jdk.incubator.vector/linux/native/libvectormath/README line 41: > 39: > 40: Currently, the only necessary change is: > 41: * make `Sleef_rempitabdp` and `Sleef_rempitabsp` in sleefinline_advsimd.h > and sleefinline_sve.h `static` to avoid multiple definitions. These lines can be removed now. - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19185#pullrequestreview-2158620004 PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1665522936 PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1665523304
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Thu, 27 Jun 2024 21:59:30 GMT, Mikael Vidstedt wrote: >> In case you need it and avoid to generate yourself, I've generated sleef >> inline header of 3.6.1 for riscv, it's at >> https://github.com/openjdk/jdk/pull/18605/commits/c279a3c290554892939267d9ebe88198988d31a4 > > @Hamlin-Li I have generated the header files (two aarch64 and the new one for > riscv64) using SLEEF 3.6.1. Please make sure they match your expectations! @vidmik I have verified the tests and performance for aarch64, it's good as before, I also updated related information in https://github.com/openjdk/jdk/pull/18605. Although have some issue for riscv64, but I think it's some env issues rather than issue in `sleefinline_rvvm1.h`. So, I think we are good to go with this pr. Thanks a lot for the work! - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2208637194
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... 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 - Changes: https://git.openjdk.org/jdk/pull/18605/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18605=08 Stats: 21668 lines in 21 files changed: 21624 ins; 1 del; 43 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Thu, 27 Jun 2024 21:59:30 GMT, Mikael Vidstedt wrote: >> In case you need it and avoid to generate yourself, I've generated sleef >> inline header of 3.6.1 for riscv, it's at >> https://github.com/openjdk/jdk/pull/18605/commits/c279a3c290554892939267d9ebe88198988d31a4 > > @Hamlin-Li I have generated the header files (two aarch64 and the new one for > riscv64) using SLEEF 3.6.1. Please make sure they match your expectations! @vidmik Thanks for updating, I think I'd better to verify it in case we need uncessary further changes. Will update when I finish. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2196492518
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Thu, 23 May 2024 16:06:42 GMT, Magnus Ihse Bursie wrote: >> I think both `sleefinline_advsimd.h` and `sleefinline_sve.h` are specific >> for arm. >> In the future, on riscv the corresponding file name will be >> `sleefinline_rvvm1.h`. >> >> Only `misc.h` is a generic file shared among platforms. > > Oh, you mean that the `sve` suffix signals that it is for ARM. I thought you > were talking about having a name like `sleefinline_aarch64.h`. > > Sure, if the only files generated by sleef are ever .h files, the logic for > chosing which to include can be done entirely by `#ifdef`s in the source > code. But if there ever needs to be different .c or .cpp files to include in > the build, the build system needs to be able to determine automatically if > they should be included or included, and that can only be made if the path or > the file name includes the CPU moniker. > > Personally, I think it would show good alignment with the prevailing norms in > the JDK to also use this way of naming files for .h files. But I confess that > for .h files it is more a matter of style, rather than a necessity from the > build system. Thanks for the information. I think the files from sleef will be .h headers only. I'm also fine to align with prevailing norms in the JDK, it's always good to do so. Just a reminder for us in the future discussion of https://github.com/openjdk/jdk/pull/18605, maybe we should consider this dir or file naming norms (e.g. currently there are vector_math_sve.c and vector_math_neon.c under src/jdk.incubator.vector/linux/native/libvectormath), as there will be more files related to different platforms added in the future, e.g. riscv64. - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1612933942
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Thu, 23 May 2024 10:40:26 GMT, Magnus Ihse Bursie wrote: >>> So you'd need a different copy of sleef for each platform? >> >> I think it's one or more. >> >>> The files you have put in linux/native/libvectormath, what platform are >>> they for? Should we not put them in a platform-specific subdirectory? >> >> we could, but not necessary, as long as they have different suffixes, and >> normally that suffixes indicate what platform it's for. > > Okay, suffix works fine too. But the files currently in the patch is named > e.g. `sleefinline_advsimd.h`, which does not indicate any platform at all. Is > it a generic file, and the platform specific ones are still missing from this > PR? I think both `sleefinline_advsimd.h` and `sleefinline_sve.h` are specific for arm. In the future, on riscv the corresponding file name will be `sleefinline_rvvm1.h`. Only `misc.h` is a generic file shared among platforms. - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611636701
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Wed, 22 May 2024 09:31:27 GMT, Magnus Ihse Bursie wrote: > So you'd need a different copy of sleef for each platform? I think it's one or more. > The files you have put in linux/native/libvectormath, what platform are they > for? Should we not put them in a platform-specific subdirectory? we could, but not necessary, as long as they have different suffixes, and normally that suffixes indicate what platform it's for. - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1609635623
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. In case you need it and avoid to generate yourself, I've generated sleef inline header of 3.6.1 for riscv, it's at https://github.com/openjdk/jdk/pull/18605/commits/c279a3c290554892939267d9ebe88198988d31a4 - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2186590880
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Fri, 17 May 2024 16:45:19 GMT, Mikael Vidstedt wrote: > Thank you Hamlin. I'll try to keep my eyes open for the announcement of the > upcoming SLEEF release but do feel free to notify me if you see it first! Thank you @vidmik , sure, I will do it. > I, too, envision that we'll be importing header files (only). That said, I'd > very much prefer *not* to rename them as part of the import. If anything I > could see us have architecture specific directories in which we place the > respective files (and a common directory for `misc.h`), but it's not > immediately clear to me that it's worth it given that the files are used in > such a narrow context in the JDK. Hi @vidmik, Sleef 3.6.1 was just released, https://github.com/shibatch/sleef/releases/tag/3.6.1, which includes the fixes https://github.com/shibatch/sleef/pull/536, https://github.com/shibatch/sleef/pull/537 which fixed the performance issue. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2119823702 PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2159052520
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v6]
On Thu, 27 Jun 2024 11:53:38 GMT, Fei Gao wrote: >> in progress... > > Hi @Hamlin-Li , thanks for your work. > > I tried to run benchmarks, > [FloatMaxVector](https://github.com/openjdk/panama-vector/blob/vectorIntrinsics/test/micro/org/openjdk/bench/jdk/incubator/vector/operation/FloatMaxVector.java#L1068) > and > [DoubleMaxVector](https://github.com/openjdk/panama-vector/blob/vectorIntrinsics/test/micro/org/openjdk/bench/jdk/incubator/vector/operation/DoubleMaxVector.java#L1068), > on different aarch64 machines. > > Here is the data I got for `TANH`, with args `-i 5 -f 3 -wi 3 -foe true > -jvmArgs -Xms4g -Xmx4g -XX:+AlwaysPreTouch -XX:ObjectAlignmentInBytes=16`: > > > // NEON machine > Benchmark (size) Mode Cnt Units Perf gain > DoubleMaxVector.TANH 1024thrpt15 ops/ms -38% > FloatMaxVector.TANH1024thrpt15 ops/ms -26% > > > > // 128-bit sve machine (TANH also implemented with NEON) > Benchmark (size) Mode Cnt Units Perf gain > DoubleMaxVector.TANH 1024thrpt15ops/ms-19% > FloatMaxVector.TANH1024thrpt15ops/ms~00% > > > The performance of vector stubs for `TANH` looks not quite stable on > different NEON machines. Since this pr does not provide `TANH` interface on > sve machines for [the performance > regression](https://github.com/openjdk/jdk/pull/16234/commits/2a7730d6acbac80438a43d1502cff6a476f8b5b5#diff-9112056f732229b18fec48fb0b20a3fe824de49d0abd41fbdb4202cfe70ad114R8521-R8525), > how about also disabling it on NEON for the same reason? WDYT? > > Thanks. @fg1417 Thanks for testing. Sure, I can do that based on your test result, I will restart work on it after https://github.com/openjdk/jdk/pull/19185 is integrated. @theRealAph I lost my previous vm, so currently I only generate the header files, but did not test performance since last time, I don't remember I had special vm options passed in at that time. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2195132669
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v8]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... Hamlin Li has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 32 commits: - 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 - minor - ... and 22 more: https://git.openjdk.org/jdk/compare/ed149062...fe4be2c6 - Changes: https://git.openjdk.org/jdk/pull/18605/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18605=07 Stats: 21668 lines in 21 files changed: 21624 ins; 1 del; 43 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v7]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... Hamlin Li has updated the pull request incrementally with two additional commits since the last revision: - sleef 3.6.1 for riscv - sleef 3.6.1 - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/36415c34..c279a3c2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=05-06 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v6]
On Wed, 8 May 2024 17:41:23 GMT, Hamlin Li 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! >> >> ## Performance >> NOTE: >> * `Src` means implementation in this pr, i.e. without depenency on external >> sleef. >> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` >> * `system_sleef` means implementation in [previous pr >> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk >> with depenency on external sleef. >> >> Basically, the perf data below shows that >> * this implementation has better performance than previous version in [pr >> 18294](https://github.com/openjdk/jdk/pull/18294), >> * and both sleef versions has much better performance compared with >> non-sleef version. >> >> |Benchmark |(size)|Src >> |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| >> |--|--|-|-||--|-|-| >> |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 >> |16799.273|0.966| >> |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 >> |16602.442|1.419| >> |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 >> |26329.320|1.292| >> |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 >> |42084.100|1.526| >> |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 >> |35998.688|0.895| >> |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 >> |24420.692|0.734| >> |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 >> |21343.863|0.749| >> |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638 ... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > update header files for arm in progress... - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2151633043
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. > Let me suggest that we wait for the next release of SLEEF to be released in > that case since that release seems to be imminent. That way we'll get both > the performance fixes _and_ we can include the RISC-V header files at the > same time. Fair? Yes, it makes sense to me. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2116848373
Re: RFR: 8329816: Add SLEEF version 3.6
On Mon, 13 May 2024 17:33:20 GMT, Mikael Vidstedt wrote: > Looks like that change is not yet in a released version of SLEEF, and in > particular not in 3.6. > > We do need updated approvals when we pick up new versions. Since we've gone > through the process once it's typically easier/faster to do so. It will be > technically easier/faster as well, now that I know how to do it and have > encoded it in the createSleef.sh script. Thanks, I see. As the version 3.6 will introduce some performance regression compared to non-intrinsic version, so to bring the fix into jdk, we need either do a manual change (like https://github.com/openjdk/jdk/pull/18605/commits/cd70f5a970514938d05f7a2426b15f78245236d3), or wait the next version after 3.6 (which should include changes in https://github.com/shibatch/sleef/pull/537), I think we prefer the latter (i.e. depends on next version after 3.6) That means https://github.com/openjdk/jdk/pull/18605 (JDK-8312425) requires sleef version next to 3.6 (could be 3.6.1 or 3.7). I'm OK to push this pr first, if it's also convenient for you, as we will need to push the change between 3.6.1/3.7 and 3.6 again when 3.6.1/3.7 is released. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2108788721
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Marked as reviewed by mli (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19185#pullrequestreview-2053815054
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote:
> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to
> optimize vector math operations by leveraging the SLEEF library. For legal
> reasons the actual contribution of the SLEEF files needs to be handled
> separately. This enhancement adds the relevant files, enabling the rest of
> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward.
Thanks for working on this.
Just one question, currently in `sleefinline_{advsimd,sve}.h` there is `#define
SLEEF_ALWAYS_INLINE inline`, but what expected is something like `#define
SLEEF_ALWAYS_INLINE inline __attribute__((always_inline))` (please check
https://github.com/shibatch/sleef/pull/537 for details). In the future, when we
change it, do we need to go through the legal process again? I guess we don't
need it, but just double check it.
-
PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2106818228
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v6]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: update header files for arm - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/bd9c0931..36415c34 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=04-05 Stats: 20 lines in 2 files changed: 14 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Tue, 9 Apr 2024 20:10:36 GMT, Mikael Vidstedt wrote: >> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - disable unused-function warnings; add log msg >> - minor > > Thank you for the update and for working on this in general. > > I've started working on JDK-8329816, preparing the change for the SLEEF > specific part of the change. Specifically, I'm currently planning on > including the three SLEEF header files, the README and a legal/sleef.md file > in that change. Let me know if you have any thoughts/concerns. > > Also, just for my understanding, would love to understand your thoughts on > the future here (I apologize if this was already discussed elsewhere): > > It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF > README mentions that "Due to limited test capacities, SLEEF is currently only > officially supported on Linux with gcc or llvm/clang." ). That same README > does, however, indicate good test coverage on several architectures in > addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it > looks like we could potentially use SLEEF for other architectures on linux in > the future? And potentially additional operating systems as well? Hey @vidmik , I just added inline header file for riscv64, hope to help avoid go through the legal process for arm and riscv header files separately. For implementation on riscv64, I will put it in another pr. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2101055673
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v5]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: add inline header file for riscv64 - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/cbcd4634..bd9c0931 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=03-04 Stats: 7073 lines in 1 file changed: 7073 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Thu, 11 Apr 2024 10:36:03 GMT, Hamlin Li 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! >> >> ## Performance >> NOTE: >> * `Src` means implementation in this pr, i.e. without depenency on external >> sleef. >> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` >> * `system_sleef` means implementation in [previous pr >> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk >> with depenency on external sleef. >> >> Basically, the perf data below shows that >> * this implementation has better performance than previous version in [pr >> 18294](https://github.com/openjdk/jdk/pull/18294), >> * and both sleef versions has much better performance compared with >> non-sleef version. >> >> |Benchmark |(size)|Src >> |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| >> |--|--|-|-||--|-|-| >> |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 >> |16799.273|0.966| >> |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 >> |16602.442|1.419| >> |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 >> |26329.320|1.292| >> |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 >> |42084.100|1.526| >> |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 >> |35998.688|0.895| >> |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 >> |24420.692|0.734| >> |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 >> |21343.863|0.749| >> |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638 ... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix performance issue Based on these 2 pr (https://github.com/shibatch/sleef/pull/537, https://github.com/shibatch/sleef/pull/536), there is no necessary code change in sleef files anymore. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2079495537
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
> 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! > > ## Performance > NOTE: > * `Src` means implementation in this pr, i.e. without depenency on external > sleef. > * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` > * `system_sleef` means implementation in [previous pr > 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk > with depenency on external sleef. > > Basically, the perf data below shows that > * this implementation has better performance than previous version in [pr > 18294](https://github.com/openjdk/jdk/pull/18294), > * and both sleef versions has much better performance compared with non-sleef > version. > > |Benchmark |(size)|Src > |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| > |--|--|-|-||--|-|-| > |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 > |16799.273|0.966| > |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 > |16602.442|1.419| > |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 > |26329.320|1.292| > |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 > |42084.100|1.526| > |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 > |35998.688|0.895| > |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 > |24420.692|0.734| > |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 > |21343.863|0.749 | > |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638|0.049 > |20155.903|3.427| > |3480:D... Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: remove notes about sleef changes - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/cd70f5a9..cbcd4634 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=02-03 Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Thu, 11 Apr 2024 10:36:03 GMT, Hamlin Li 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! >> >> ## Performance >> NOTE: >> * `Src` means implementation in this pr, i.e. without depenency on external >> sleef. >> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` >> * `system_sleef` means implementation in [previous pr >> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk >> with depenency on external sleef. >> >> Basically, the perf data below shows that >> * this implementation has better performance than previous version in [pr >> 18294](https://github.com/openjdk/jdk/pull/18294), >> * and both sleef versions has much better performance compared with >> non-sleef version. >> >> |Benchmark |(size)|Src >> |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| >> |--|--|-|-||--|-|-| >> |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 >> |16799.273|0.966| >> |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 >> |16602.442|1.419| >> |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 >> |26329.320|1.292| >> |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 >> |42084.100|1.526| >> |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 >> |35998.688|0.895| >> |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 >> |24420.692|0.734| >> |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 >> |21343.863|0.749| >> |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638 ... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix performance issue I've also updated the pr description with performance data, it shows that * this implementation has better performance than previous version in https://github.com/openjdk/jdk/pull/18294, * and both sleef versions has much better performance compared with non-sleef version. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049482861
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Thu, 11 Apr 2024 10:36:03 GMT, Hamlin Li 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! > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix performance issue Thanks everyone for discussion about the direction (integrate source or lib). We did have some implementation for integrating sleef lib into jdk, but seems previously the most strong opinion is to integrate the sleef source into jdk. I know there are cons and pros for every solution, but I will stick to current solution unless everyone can reach another agreement. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049410731
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Tue, 9 Apr 2024 20:10:36 GMT, Mikael Vidstedt wrote: >> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - disable unused-function warnings; add log msg >> - minor > > Thank you for the update and for working on this in general. > > I've started working on JDK-8329816, preparing the change for the SLEEF > specific part of the change. Specifically, I'm currently planning on > including the three SLEEF header files, the README and a legal/sleef.md file > in that change. Let me know if you have any thoughts/concerns. > > Also, just for my understanding, would love to understand your thoughts on > the future here (I apologize if this was already discussed elsewhere): > > It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF > README mentions that "Due to limited test capacities, SLEEF is currently only > officially supported on Linux with gcc or llvm/clang." ). That same README > does, however, indicate good test coverage on several architectures in > addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it > looks like we could potentially use SLEEF for other architectures on linux in > the future? And potentially additional operating systems as well? Hey, @vidmik I've fixed the performance issue, and update the sleef inline headers and README. It's good for you to integrate these files via JDK-8329816. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049400793
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
> 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! Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: fix performance issue - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/34529ff1..cd70f5a9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=01-02 Stats: 4 lines in 4 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Wed, 10 Apr 2024 09:24:09 GMT, Hamlin Li wrote: > > Thank you for the update and for working on this in general. > > I've started working on JDK-8329816, preparing the change for the SLEEF > > specific part of the change. Specifically, I'm currently planning on > > including the three SLEEF header files, the README and a legal/sleef.md > > file in that change. Let me know if you have any thoughts/concerns. > > Thanks a lot, that's a great news. Please go ahead to integrate the files via > JDK-8329816. :) Besides of the performance issue currently found out, I have > no other concerns. I found the root cause of the performance regression, and have a draft solution for it, I'm running a thorough benchmark to see if it works for all sleef functions we use in jdk. So, basically this solution is good. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2048217671
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Tue, 9 Apr 2024 20:10:36 GMT, Mikael Vidstedt wrote: > Thank you for the update and for working on this in general. > > I've started working on JDK-8329816, preparing the change for the SLEEF > specific part of the change. Specifically, I'm currently planning on > including the three SLEEF header files, the README and a legal/sleef.md file > in that change. Let me know if you have any thoughts/concerns. > Thanks a lot, that's a great news. Please go ahead to integrate the files via JDK-8329816. :) Besides of the performance issue currently found out, I have no other concerns. > Also, just for my understanding, would love to understand your thoughts on > the future here (I apologize if this was already discussed elsewhere): > > It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF > README mentions that "Due to limited test capacities, SLEEF is currently only > officially supported on Linux with gcc or llvm/clang." ). That same README > does, however, indicate good test coverage on several architectures in > addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it > looks like we could potentially use SLEEF for other architectures on linux in > the future? And potentially additional operating systems as well? There are more informantion at https://sleef.org/compile.xhtml, seems it could be formally supported in the future, but I'm not sure about it. Maybe others have more information could help to comment here. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2047008550
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Fri, 5 Apr 2024 12:17:17 GMT, Hamlin Li 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! > > Hamlin Li has updated the pull request incrementally with two additional > commits since the last revision: > > - disable unused-function warnings; add log msg > - minor Just a quick update, this pr introduces some performance regression compared with previous version (https://github.com/openjdk/jdk/pull/18294) for some math functions (e.g. Double256Vector.COS), and no regression for some others (e.g. Double256Vector.ACOS). I'm investigating. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2045495626
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Fri, 5 Apr 2024 10:45:24 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > Use JNIEXPORT Looks good. Although I'm not the right person to review this, thanks for explanation and discussion. - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18400#pullrequestreview-1988652730
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 4 Apr 2024 21:55:54 GMT, Mikael Vidstedt wrote: >> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - disable unused-function warnings; add log msg >> - minor > > make/modules/jdk.incubator.vector/Lib.gmk line 44: > >> 42: $(eval $(call SetupJdkLibrary, BUILD_LIBVECTORMATH, \ >> 43: NAME := vectormath, \ >> 44: CFLAGS := $(CFLAGS_JDKLIB) -Wno-error=unused-function, \ > > Should the unused-function be passed in using `DISABLE_WARNINGS_*` instead? Thanks! Good suggestion, it makes the output clean. - PR Review Comment: https://git.openjdk.org/jdk/pull/18605#discussion_r1553519958
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
On Thu, 4 Apr 2024 16:47:44 GMT, Magnus Ihse Bursie wrote: > Build libsleef using their cmake system and look at the compile command line. > (You do this by `VERBOSE=1 cmake` IIRC). Then you can see what flags they are > using. This is what I was referring to as "normal libsleef build". I noticed > there were a lot of compiler flags. I can't say if they are needed or not. In > most cases, if it compilers, it's fine, but in this case, I guess some flags > can be crucial to really get the kind of performance you need, and it might > not be easy to spot that something is wrong if you get them incorrect. I > assume one way to make sure is to run microbenchmarks with an externally > built libsleef and compare it with the one you build within the JDK. If there > is no noticeable difference, then I guess it is fine. Thanks for the clarification and good suggestion. I will verify it and update here later. Just right now I have some trouble to get an aarch64 linux, I tried to get a graviton instance on AWS, but I failed to connect it when I create it. Previously I run all the test for correctness via qemu, but seems qemu is not for performance test. So I will update later when I get the environment ready. If someone got the easy environment to verify the performance, it's very welcome. :) - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2039645463
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
> 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! Hamlin Li has updated the pull request incrementally with two additional commits since the last revision: - disable unused-function warnings; add log msg - minor - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/3ab4795d..34529ff1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=00-01 Stats: 8 lines in 4 files changed: 5 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
On Wed, 3 Apr 2024 19:23:01 GMT, Magnus Ihse Bursie wrote: > Just a quick question after giving this a glance: My understanding was that > the normal libsleef build set a lot of compiler options, e.g. disabling > built-in maths etc. You don't seem to set any of these. Have you determined > that they were not needed? Thanks for having a look and quick response. Good question. Per `disabling built-in maths`, my understanding is that maybe we don't need to care about it, as this built-in math functions in compilers are only for scalar version, but we're using sleef's simd versions only which use vector intrinsics I think. e.g. in `src/libm/sleefdp.c` there is `ENABLE_BUILTIN_MATH` check, but in `src/libm/sleefsimdsp.c` there is no such check, so when generating inline header files, I assume its value (whether enable/disable built-in math) does not impact the generated simd functions. Please correct me if I'm understanding it wrongly. For other compiler options, I tend to agree with you, but I'm not sure which might need, can you supply more information or point to some reference about `normal libsleef build`? BTW, what I refered to before was from sleef.org and sleef on github (including its github workflow). - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2037072600
Withdrawn: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
On Thu, 14 Mar 2024 08:48:11 GMT, Hamlin Li wrote: > Hi, > Can you help to review this patch? > Thanks > > This is a continuation of work based on [1] by @XiaohongGong, most work was > done in that pr. In this new pr, just rebased the code in [1], then added > some minor changes (renaming of calling method, add libsleef as extra lib in > CI cross-build on aarch64 in github workflow); I aslo tested the combination > of following scenarios: > * at build time > * with/without sleef > * with/without sve support > * at runtime > * with/without sleef > * with/without sve support > > [1] https://github.com/openjdk/jdk/pull/16234 > > ## Regression Test > * test/jdk/jdk/incubator/vector/ > * test/hotspot/jtreg/compiler/vectorapi/ > > ## Performance Test > Previously, @XiaohongGong has shared the data: > https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/18294
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes Hey everyone, Please review the change at https://github.com/openjdk/jdk/pull/18605 when you're available. This pr will be closed. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2034835163
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 28 Mar 2024 18:41:03 GMT, Paul Sandoz wrote: >> Hamlin Li has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix jni includes > > Hamlin, thank you for working on this. I think integrating a sub-set of SLEEF > is valuable (not all of it makes sense e.g., DFT part). My recommendation > would be to focus on a PR that integrates the required source, rather than > taking steps towards that. > > AFAICT from browsing prior comments "integrate the source" appears to be the > generally preferred solution, but there is some understandable hesitancy > about legal aspects. IIUC from what you say this is a technically feasible > and maintainable solution. As said here: > >> We (Oracle Java Platform Group) can handle the required "paperwork > https://github.com/openjdk/jdk/pull/16234#issuecomment-1823335443 > Thanks @PaulSandoz for your comment and suggestion. > > I will work on the solution which integrates the source of sleef into jdk. I've created a new pr at https://github.com/openjdk/jdk/pull/18605 which is to integrate the sleef source into jdk so to avoid depending on external sleef at build or run time. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2034832611
RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
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! - Commit messages: - minor - add maintenance nodes - merge master - remove unnecessary changes - resolve build erorrs - add [generated] src from sleef - fix jni includes - rename - resolve magicus's comments - fix variable name in github workflow - ... and 13 more: https://git.openjdk.org/jdk/compare/6ae1cf12...3ab4795d Changes: https://git.openjdk.org/jdk/pull/18605/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18605=00 Issue: https://bugs.openjdk.org/browse/JDK-8312425 Stats: 14582 lines in 20 files changed: 14535 ins; 1 del; 46 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 28 Mar 2024 18:41:03 GMT, Paul Sandoz wrote: >> Hamlin Li has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix jni includes > > Hamlin, thank you for working on this. I think integrating a sub-set of SLEEF > is valuable (not all of it makes sense e.g., DFT part). My recommendation > would be to focus on a PR that integrates the required source, rather than > taking steps towards that. > > AFAICT from browsing prior comments "integrate the source" appears to be the > generally preferred solution, but there is some understandable hesitancy > about legal aspects. IIUC from what you say this is a technically feasible > and maintainable solution. As said here: > >> We (Oracle Java Platform Group) can handle the required "paperwork > https://github.com/openjdk/jdk/pull/16234#issuecomment-1823335443 Thanks @PaulSandoz for your comment and suggestion. I will work on the solution which integrates the source of sleef into jdk. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2031671597
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes Hello @PaulSandoz, For various reasons, [previous pr](https://github.com/openjdk/jdk/pull/16234) and current pr are thought not good enough, so I did some exploration of [integrating sleef source into jdk](https://github.com/openjdk/jdk/pull/18294#issuecomment-2018839778), the solution itself is simple and maintainace is easy. I saw in the previous discussion, you had some [comments about integrate sleef source into jdk](https://github.com/openjdk/jdk/pull/16234#issuecomment-1821885433). I'm not sure if you're avaialbe to take a look and continue the discussion and share your opinions? If positive, we can start from [here](https://github.com/openjdk/jdk/pull/18294#issuecomment-2020986186). Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2023136169
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Wed, 27 Mar 2024 14:47:00 GMT, Magnus Ihse Bursie wrote: > At this point, I think I am not really qualified to continue the discussion. > > I will ask around inside Oracle to see if I can find someone who better > understands the implications of the suggested libsleef integration (with or > without source code incorporation). I can return when there is some common > understanding on how to proceed, if that will lead to any build system > changes. OK, Thanks for discussion and reviewing! - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2023113752
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes OK, no worry. Let's discuss more thoroughly before we start implement it. Let me ask first question: are we still want sleef into jdk? * If the answer is `no` or similar, what's the better alternative solution? * If the answer is `yes`, let's continue the discussion about how to integrate sleef into jdk via this pr (maybe not imeplement it in this pr). I try to categorize the possible solutions below. 1. depends on external sleef at build time & run time, like this pr. Cons: requires sleef installed in system at build & run time. 2. depends on external sleef at run time only, like @theRealAph suggested. Cons: requires sleef installed in system at runtime 3. depends on external sleef at build time only, like @magicus suggested. Cons: requires sleef installed in system at build time 4. not depends on external sleef at all. 4.1. integrate source into jdk, use the genrated source from sleef cmake, check https://github.com/openjdk/jdk/pull/18294#issuecomment-2018839778, Cons: license legal process 4.2. is there other ways? 6. any other possible solutions not covered above? Seems to me, the best way is to integrate sleef souce into jdk. Does it need license work? If positive, how and who will work on it? Would you mind to share your comments, and make necessary additions? @theRealAph @magicus Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2020986186
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes OK, I'm fine with it. I just hope we could have made this decision earlier, as I have asked this question before: https://github.com/openjdk/jdk/pull/16234#issuecomment-1973345855, and at that time seems there is no "No" answer. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2020859623
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 25 Mar 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: > > A question about the licensing: how long does it take to finish the legal > > process of the sleef licence? > > I am not sure this is a relevant question anymore. As I said, the libsleef > build system seems virtually impossible to integrate into the OpenJDK build, > so I no longer believe this is a way forward. It's not necessary to integrate libsleef build sysstem into jdk, we just need to integrate the final sources (generated by sleef cmake) into jdk. I have tested it, it works. We still need legal process support from Oracle. But anyway, it's a furture incremental solution after this pr, am I right? Or are we going to change direction? - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2018839778
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 25 Mar 2024 09:24:16 GMT, Magnus Ihse Bursie wrote: > > > But that raises an interesting question. What happens if you try to load > > > a library compiled with `-march=armv8-a+sve` on a non-SVE system? Is the > > > ELF flagged to require SVE so it will fail to load? I'm hoping this is > > > the case -- if so, everything will work as it should in this PR, but I > > > honestly don't know. (After spending like 10 years working with building, > > > I still discover things I need to learn...). > > > > > > I think we can handle it, when a jdk built with sve support runs on a > > non-sve machine, the sve related code will not be executed with the > > protection of UseSVE vm flag which is detected at runtime startup. > > You misunderstand me; perhaps I'm not clear enough here. > > First of all, my question was of a more general nature. Is there such a > mechanism in the dynamic linker that protects us from loading libraries that > will fail if an ISA extension is used that is missing on the running system? > Or do the linker just check that the ELF is for the right CPU? IMHO, Ithe dymic linker will not check ISA extension. > > Secondly, I assume that libsleef.so proper needs to be compiled with SVE > support as well. So if we were to skip the shim vectormath library and load > libsleef directly from hotspot, what would happen then? > > Thirdly, the check with UseSVE happens _after_ you load the library. If there > is a DL verification, you will not even reach this check, but get a DLL > loading failure before that. Sure, regardless of which happens, you will not > execute bad code, but I'd like to know which is the case. In that situation, I think UseSVE check will return false to avoid set functions pointer (e.g. StubRoutines::_vector_f_math[VectorSupport::VEC_SIZE_SCALABLE][op]) when running on non-SVE matchine. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2018827762
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 22 Mar 2024 10:29:36 GMT, Robbin Ehn wrote: >>> What is the relevance of SVE support at build time? Should it matter what >>> the build machine is? >> >> My understanding is that we need a compiler that supports >> `-march=armv8-a+sve`; otherwise we can't build the redirect library >> properly. But maybe that is a misunderstanding? > >> > What is the relevance of SVE support at build time? Should it matter what >> > the build machine is? >> >> My understanding is that we need a compiler that supports >> `-march=armv8-a+sve`; otherwise we can't build the redirect library >> properly. But maybe that is a misunderstanding? > > Yes, it should be in gcc 8 and above, and [we seem to require gcc > 10](https://openjdk.org/groups/build/doc/building.html). > @robehn Great! I guess that option goes hand-in-hand with arm_sve.h? If so, > we can remove the complication of the SVE check completely. If we all agree to optimistically integrate the current pr (needs both build time + run time sleef support) first, I can check related things, and do necessary modification/improvement. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2015353690
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 22 Mar 2024 12:42:04 GMT, Magnus Ihse Bursie wrote: > > Ah, it'll only be the redirect library that's compiled with > > -march=armv8-a+sve Forget that. > > But that raises an interesting question. What happens if you try to load a > library compiled with `-march=armv8-a+sve` on a non-SVE system? Is the ELF > flagged to require SVE so it will fail to load? I'm hoping this is the case > -- if so, everything will work as it should in this PR, but I honestly don't > know. (After spending like 10 years working with building, I still discover > things I need to learn...). I think we can handle it, when a jdk built with sve support runs on a non-sve machine, the sve related code will not be executed with the protection of UseSVE vm flag which is detected at runtime startup. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2015348458
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes > Some updates: I had a look at the source code for libsleef, and frankly, to > integrate building of libsleef into the OpenJDK build is going to be a major > PITA. :-( They have a cmake based build system, which generates a ton of > native build tools, which preprocess source code and generates output > artifacts (like sleef.h). > > I guess we _could_ do it, but I am terrified about trying to get that to > work, especially for cross-compilation. > > So after seeing this, I think the better solution is to require a > pre-compiled libsleef present in the system. (What I just called the "worse > solution" an hour ago...). I guess we can make the requirement of libsleef > the default value, but allow the user to override this to skip libsleef. At > least then it is an active choice to disable libsleef for your build. > > Or, we could do something like how we used to do for freetype: have a > --with-libsleef-src that points to a checked out version of libsleef, and let > configure build it. In this case we could even build it as a static archive, > so there'd be no need for libvectormath.so to have any external dependencies. > > In any case, we are going to have to consider carefully how to proceed. No worry, seems works for me, I just had a very draft version tested locally. we just need to get the final inline versions of sleef generated by sleef cmake install. A question about the licensing: how long does it take to finish the legal process of the sleef licence? - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2015341870
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 22 Mar 2024 10:29:36 GMT, Robbin Ehn wrote: > > > What is the relevance of SVE support at build time? Should it matter what > > > the build machine is? > > > > > > My understanding is that we need a compiler that supports > > `-march=armv8-a+sve`; otherwise we can't build the redirect library > > properly. But maybe that is a misunderstanding? > > Yes, it should be in gcc 8 and above, and [we seem to require gcc > 10](https://openjdk.org/groups/build/doc/building.html). For ACLE, it's supported in [10.1](https://gcc.gnu.org/gcc-10/changes.html) - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2015334896
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 21 Mar 2024 15:43:28 GMT, Magnus Ihse Bursie wrote: >>> > The problem I see is that J. Random Java User has no way to know if SLEEF >>> > is making their program faster without running benchmarks. They'll put >>> > SLEEF somewhere and hope that Java uses it. >>> >>> Please kindly correct me if I misunderstood your points. Seems the safest >>> solution to address your above concerns is to integrate the sleef source >>> into jdk? Lack of sleef at either build time or runtime will make the >>> user's code fall back to java implementation. >> >> Exactly, yes. That's why we've integrated the source code of many other >> libraries we depend on into the JDK. It's the only way to get the >> reliability our users expect. > > @theRealAph Are you saying that bundling the source code of libsleef is a > hard requirement from your side to accept this code into the JDK? > > I'm not against it, I just want to understand what we're talking about here. > > In general, adding new libraries to OpenJDK will require a legal process in > Oracle, which may (or may not) take some amount of time T, where T is larger > than you'd wish for. > > So I guess we can either: > 1) wait for libsleef source code to become a part of OpenJDK, and then > integrate this PR. > 2) integrate this PR optimistically, and in the background start a process of > trying to get libsleef into OpenJDK. (Which, of course, can not be 100% > guaranteed to happen.) > 3) integrate this PR as is, and give up any idea of bundling libsleef. > > I also believe there is a fourth option, but that too seems like it has legal > implications that needs to be checked: > > 4) if the libsleef dynamic library is found on the system during build time, > bundle a copy of the dll with the built JDK. (Similar to how was done with > freetype on Windows before.). And, optionally, provide an option for > configure to require libsleef to be present, so the build fails if no > libsleef can be found and bundled. (Leaving open as to if this should be > default or not.) @magicus @theRealAph Thanks for discussion. > I think that's probably best the best thing to do today. I will approve it. Great, Thanks. > I'd prefer, long term, to integrate SLEEF into OpenJDK. That would make > AArch64 support similar to Intel. We are competing with Intel. I'm working on it, running some tests. But I think as @magicus pointed out, to achieve this target one of most important tasks is to get the legal process done. > In the short term, I'd build a shim library during the default standard JDK > build that does not need SLEEF at build time. Unless weo do that, because > SLEEF isn't on anyone's builders, it won't be used. I agree it's give user more chance to leverage the sleef when running, but I wonder if it's necessary to do that. As we have a long term solution, and the chance that end user lacks sleef library in their environment is much higher than release engineer lacks sleef library in their environment. But it's not harm to do so. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2013123992
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Fri, 15 Mar 2024 11:25:37 GMT, Hamlin Li wrote: > > * There is no way to stop the VM from trying to load vmath ? > > No official way, but deleting libvmath.so will have a same effect. I'm not > sure if avoiding loading vmath is necessary for typical users, if it turns > out to be true, we can add it later. In previous implementation of introducing SVML into jdk on x86, it introduced a option `UseVectorStubs` which can be used to avoid using libvmath(either svml or sleef), i.e. user can fall back to java implementation if they want. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2009399949
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review this patch? >> Thanks >> >> This is a continuation of work based on [1] by @XiaohongGong, most work was >> done in that pr. In this new pr, just rebased the code in [1], then added >> some minor changes (renaming of calling method, add libsleef as extra lib in >> CI cross-build on aarch64 in github workflow); I aslo tested the combination >> of following scenarios: >> * at build time >> * with/without sleef >> * with/without sve support >> * at runtime >> * with/without sleef >> * with/without sve support >> >> [1] https://github.com/openjdk/jdk/pull/16234 >> >> ## Regression Test >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ## Performance Test >> Previously, @XiaohongGong has shared the data: >> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix jni includes > If there was some kind of plan, with evidence of the intention to do > something to get this valuable tech into people's hands in a form they can > use, sure. But as you can tell, I think this may rot because no one will be > able use it. If SLEEF were included in the JDK it'd be fine. if SLEEF were a > common Linux library it'd be fine. > > The problem I see is that J. Random Java User has no way to know if SLEEF is > making their program faster without running benchmarks. They'll put SLEEF > somewhere and hope that Java uses it. Please kindly correct me if I misunderstood your points. Seems the safest solution to address your above concerns is to integrate the sleef source into jdk? Lack of sleef at either build time or runtime will make the user's code fall back to java implementation. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2007692746
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Fri, 15 Mar 2024 11:55:14 GMT, Magnus Ihse Bursie wrote: >> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - rename >> - resolve magicus's comments > > src/jdk.incubator.vector/linux/native/libvectormath/vector_math_neon.c line > 25: > >> 23: >> 24: #include >> 25: #include > > You should not include the "_md" (machine dependent) part of jni directly. > > Suggestion: > > #include Thanks, fixed. - PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1526325331
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
> Hi, > Can you help to review this patch? > Thanks > > This is a continuation of work based on [1] by @XiaohongGong, most work was > done in that pr. In this new pr, just rebased the code in [1], then added > some minor changes (renaming of calling method, add libsleef as extra lib in > CI cross-build on aarch64 in github workflow); I aslo tested the combination > of following scenarios: > * at build time > * with/without sleef > * with/without sve support > * at runtime > * with/without sleef > * with/without sve support > > [1] https://github.com/openjdk/jdk/pull/16234 > > ## Regression Test > * test/jdk/jdk/incubator/vector/ > * test/hotspot/jtreg/compiler/vectorapi/ > > ## Performance Test > Previously, @XiaohongGong has shared the data: > https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: fix jni includes - Changes: - all: https://git.openjdk.org/jdk/pull/18294/files - new: https://git.openjdk.org/jdk/pull/18294/files/7b7ec312..d0ed0323 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18294=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18294=02-03 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18294/head:pull/18294 PR: https://git.openjdk.org/jdk/pull/18294
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 14 Mar 2024 15:29:51 GMT, Andrew Haley wrote: > > Hi, thanks for continuing with this. > > As this is similar to SVML, comments applies to x86 also: > > ``` > > * There is no way to stop the VM from trying to load vmath ? > > There is both a UseNeon and a UseSVE, if I set both to false I would > > expect no vector and no vmath. > > ``` > > The JVM won't work without Neon. It's always there. Better to delete the flag. Sure, will track this in a seperate [pr](https://bugs.openjdk.org/browse/JDK-8328265). - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-1999469411
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 14 Mar 2024 15:25:54 GMT, Andrew Haley wrote: > > ``` > > * at build time > > > > * with/without sleef > > * with/without sve support > > ``` > > What is the relevance of SVE support at build time? Should it matter what the > build machine is? > > Its important to realize that almost no one except the JDK devs builds their > own JDK, and having SLEEF dependencies at build time will mean that almost no > one will use it. All this work you've done will be for nothing. I agree. On the other hand, I see previously there was discussion about this already, and current solution got some points https://github.com/openjdk/jdk/pull/16234#issuecomment-1836266314, https://github.com/openjdk/jdk/pull/16234#issuecomment-1836449876 So, currently maybe we could go with this solution first, and as an incremental optimization, we could introduce a dynamic loading without dependency on the bridge furnctions in libvmath at runtime. How do you think about it? > I'm running the tests, but I have no way to confirm that SLEEF or SVE is in > use. Exactly what did you do, and how did you confirm it? Thanks for running test. I just turn on some log, and check the output. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-1999468059 PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-1999470911
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 14 Mar 2024 12:23:17 GMT, Magnus Ihse Bursie wrote: >> Hamlin Li has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix variable name in github workflow > > src/jdk.incubator.vector/linux/native/libvmath/vect_math.h line 1: > >> 1: /* > > I'd just like to raise the question of naming. Right now the terms "vmath", > "vect_math" and "vector_math" seems to be used interchangeably. I think it > would be good to standardize on one name, and my suggestion is to go with the > complete name -- that fits with a general theme in Java. It has the benefit > that if there are many ways to abbreviate a term, there is only one way to > not abbreviate it. > > On the other hand, using `_` in library names is not really that common, so > I'd suggest this file should be named `libvectormath/vector_math.h`. (And > correspondingly for the other files.) Thanks for the reviewing and suggestion. All Fixed - PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1526144596
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 14 Mar 2024 15:29:51 GMT, Andrew Haley wrote: > Hi, thanks for continuing with this. Thanks for the comments > As this is similar to SVML, comments applies to x86 also: > > * There is no way to stop the VM from trying to load vmath ? No official way, but deleting libvmath.so will have a same effect. I'm not sure if avoiding loading vmath is necessary for typical users, if it turns out to be true, we can add it later. > There is both a UseNeon and a UseSVE, if I set both to false I would expect > no vector and no vmath. > The issue with UseNeon not really guarding neon code, but just crc, seems > like a pre-existing bug. > A flag like 'UseNeon' should turn it on/off :) I think Andrew asked this question, I agree it's better to remove the flag usage in crc32 on aarch64, better to be done in a separate [pr](https://bugs.openjdk.org/browse/JDK-8328265). > * Doing open dll form stubrountines seems wrong. I agree, logically it brings more clear code structure. But as this will be common change (e.g. on both x86 and aarch64) in the stub initialization steps, , and I think we'd better do it in a seprate [pr](https://bugs.openjdk.org/browse/JDK-8328266). - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-1999457249
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
> Hi, > Can you help to review this patch? > Thanks > > This is a continuation of work based on [1] by @XiaohongGong, most work was > done in that pr. In this new pr, just rebased the code in [1], then added > some minor changes (renaming of calling method, add libsleef as extra lib in > CI cross-build on aarch64 in github workflow); I aslo tested the combination > of following scenarios: > * at build time > * with/without sleef > * with/without sve support > * at runtime > * with/without sleef > * with/without sve support > > [1] https://github.com/openjdk/jdk/pull/16234 > > ## Regression Test > * test/jdk/jdk/incubator/vector/ > * test/hotspot/jtreg/compiler/vectorapi/ > > ## Performance Test > Previously, @XiaohongGong has shared the data: > https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 Hamlin Li has updated the pull request incrementally with two additional commits since the last revision: - rename - resolve magicus's comments - Changes: - all: https://git.openjdk.org/jdk/pull/18294/files - new: https://git.openjdk.org/jdk/pull/18294/files/668d08ce..7b7ec312 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18294=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18294=01-02 Stats: 63 lines in 7 files changed: 0 ins; 39 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/18294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18294/head:pull/18294 PR: https://git.openjdk.org/jdk/pull/18294
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Thu, 14 Mar 2024 08:59:09 GMT, Ludovic Henry wrote:
>> Hamlin Li has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix variable name in github workflow
>
> .github/workflows/build-cross-compile.yml line 138:
>
>> 136: --arch=${{ matrix.debian-arch }}
>> 137: --verbose
>> 138:
>> --include=fakeroot,symlinks,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxrandr-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libasound2-dev,libfreetype-dev,libpng-dev,${{
>> extra-libs }}
>
> This will have to be `${{ matrix.extra-libs }}`
Thanks, fixed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1524497586
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
> Hi, > Can you help to review this patch? > Thanks > > This is a continuation of work based on [1] by @XiaohongGong, most work was > done in that pr. In this new pr, just rebased the code in [1], then added > some minor changes (renaming of calling method, add libsleef as extra lib in > CI cross-build on aarch64 in github workflow); I aslo tested the combination > of following scenarios: > * at build time > * with/without sleef > * with/without sve support > * at runtime > * with/without sleef > * with/without sve support > > [1] https://github.com/openjdk/jdk/pull/16234 > > ## Regression Test > * test/jdk/jdk/incubator/vector/ > * test/hotspot/jtreg/compiler/vectorapi/ > > ## Performance Test > Previously, @XiaohongGong has shared the data: > https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: fix variable name in github workflow - Changes: - all: https://git.openjdk.org/jdk/pull/18294/files - new: https://git.openjdk.org/jdk/pull/18294/files/55b118d9..668d08ce Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18294=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18294=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18294/head:pull/18294 PR: https://git.openjdk.org/jdk/pull/18294
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Fri, 1 Mar 2024 15:10:30 GMT, Magnus Ihse Bursie wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix potential attribute issue > > Iirc, your assessment is right; the code should be ready for integration; I > just don't know about the state of testing. Hi @magicus @theRealAph @dholmes-ora, I've created the new pr to continue the work at: https://github.com/openjdk/jdk/pull/18294. Please take another look. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1996902143
RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
Hi, Can you help to review this patch? Thanks This is a continuation of work based on [1] by @XiaohongGong, most work was done in that pr. In this new pr, just rebased the code in [1], then added some minor changes (renaming of calling method, add libsleef as extra lib in CI cross-build on aarch64 in github workflow); I aslo tested the combination of following scenarios: * at build time * with/without sleef * with/without sve support * at runtime * with/without sleef * with/without sve support [1] https://github.com/openjdk/jdk/pull/16234 ## Regression Test * test/jdk/jdk/incubator/vector/ * test/hotspot/jtreg/compiler/vectorapi/ ## Performance Test Previously, @XiaohongGong has shared the data: https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028 - Commit messages: - copyright date - distinguish call names between non-scalable and scalable(e.g. sve) - add libsleef as extra lib in CI cross-build on aarch64 - Merge branch 'master' into sleef-aarch64 - Fix potential attribute issue - Remove -fvisibility in makefile and add the attribute in source code - Add "--with-libsleef-lib" and "--with-libsleef-include" options - Separate neon and sve functions into two source files - Rename vmath to sleef in configure - Address review comments in build system - ... and 3 more: https://git.openjdk.org/jdk/compare/de428daf...55b118d9 Changes: https://git.openjdk.org/jdk/pull/18294/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18294=00 Issue: https://bugs.openjdk.org/browse/JDK-8312425 Stats: 600 lines in 23 files changed: 548 ins; 1 del; 51 mod Patch: https://git.openjdk.org/jdk/pull/18294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18294/head:pull/18294 PR: https://git.openjdk.org/jdk/pull/18294
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Thu, 7 Dec 2023 09:30:01 GMT, Xiaohong Gong wrote: >> Currently the vector floating-point math APIs like >> `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, >> which causes large performance gap on AArch64. Note that those APIs are >> optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. >> To close the gap, we would like to optimize these APIs for AArch64 by >> calling a third-party vector library called libsleef [2], which are >> available in mainstream Linux distros (e.g. [3] [4]). >> >> SLEEF supports multiple accuracies. To match Vector API's requirement and >> implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA >> instructions used stubs in libsleef for most of the operations by default, >> and 2) add the vector calling convention to apply with the runtime calls to >> stub code in libsleef. Note that for those APIs that libsleef does not >> support 1.0 ULP, we choose 0.5 ULP instead. >> >> To help loading the expected libsleef library, this patch also adds an >> experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. >> People can use it to denote the libsleef path/name explicitly. By default, >> it points to the system installed library. If the library does not exist or >> the dynamic loading of it in runtime fails, the math vector ops will >> fall-back to use the default scalar version without error. But a warning is >> printed out if people specifies a nonexistent library explicitly. >> >> Note that this is a part of the original proposed patch in panama-dev [5], >> just with some initial review comments addressed. And now we'd like to get >> some wider feedbacks from more hotspot experts. >> >> [1] https://github.com/openjdk/jdk/pull/3638 >> [2] https://sleef.org/ >> [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/ >> [4] https://packages.debian.org/bookworm/libsleef3 >> [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Fix potential attribute issue Just a quick update, I've rebased the code, and will continue the work soon. I've looked through the previous discussion, seems there is no blocking issues, if I misunderstood or miss some information, please feel free to let me know, also if you have any further comment. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1973345855
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Thu, 7 Dec 2023 09:30:01 GMT, Xiaohong Gong wrote: >> Currently the vector floating-point math APIs like >> `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, >> which causes large performance gap on AArch64. Note that those APIs are >> optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. >> To close the gap, we would like to optimize these APIs for AArch64 by >> calling a third-party vector library called libsleef [2], which are >> available in mainstream Linux distros (e.g. [3] [4]). >> >> SLEEF supports multiple accuracies. To match Vector API's requirement and >> implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA >> instructions used stubs in libsleef for most of the operations by default, >> and 2) add the vector calling convention to apply with the runtime calls to >> stub code in libsleef. Note that for those APIs that libsleef does not >> support 1.0 ULP, we choose 0.5 ULP instead. >> >> To help loading the expected libsleef library, this patch also adds an >> experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. >> People can use it to denote the libsleef path/name explicitly. By default, >> it points to the system installed library. If the library does not exist or >> the dynamic loading of it in runtime fails, the math vector ops will >> fall-back to use the default scalar version without error. But a warning is >> printed out if people specifies a nonexistent library explicitly. >> >> Note that this is a part of the original proposed patch in panama-dev [5], >> just with some initial review comments addressed. And now we'd like to get >> some wider feedbacks from more hotspot experts. >> >> [1] https://github.com/openjdk/jdk/pull/3638 >> [2] https://sleef.org/ >> [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/ >> [4] https://packages.debian.org/bookworm/libsleef3 >> [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Fix potential attribute issue I could also take a look at it by the end of this month if no one are going to do it. - PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1932188622
Re: RFR: 8319165: hsdis binutils: warns on empty string as option string [v2]
On Wed, 1 Nov 2023 11:52:20 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> insn_options[0] is set to empty string if there is no options (NULL or empty >> strings). >> Checking it for empty string should cover both cases, caller option is NULL >> or caller option is empty string. >> >> Tested hsdis no longer gives me the warning. > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > More clear LGTM, Thanks. - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16433#pullrequestreview-1708250640
Re: RFR: 8319165: hsdis binutils: warns on empty string as option string
On Tue, 31 Oct 2023 13:05:44 GMT, Robbin Ehn wrote: > Hi, please consider. > > insn_options[0] is set to empty string if there is no options (NULL or empty > strings). > Checking it for empty string should cover both cases, caller option is NULL > or caller option is empty string. > > Tested hsdis no longer gives me the warning. src/utils/hsdis/binutils/hsdis-binutils.c line 340: > 338: native_bfd, > 339: /* On some archs we get warnings, if we > pass empty options */ > 340: (app_data->insn_options[0] == '\0') ? > NULL : app_data->insn_options); Looks good, but to me, it's more clear to keep `caller_options == NULL` too. - PR Review Comment: https://git.openjdk.org/jdk/pull/16433#discussion_r1377670049
Re: RFR: 8318027: Support alternative name to jdk.internal.vm.compiler [v4]
On Sat, 21 Oct 2023 10:56:01 GMT, Doug Simon wrote: >> The Graal code base has >> [renamed](https://github.com/oracle/graal/commit/1e41203d10db321f86723eac90f6cd0573b08b33) >> its module to `jdk.compiler.graal` as part of preparations for Project >> Galahad. Due to the way Java modules work, this requires a JDK change. The >> core of the issue is that the >> [service](https://github.com/openjdk/jdk/blob/56aa1e8dc8047cbc29d554889c64beb6eca0b8eb/src/jdk.internal.vm.ci/share/classes/module-info.java#L37) >> by which HotSpot requests a Graal compilation is defined in JVMCI. Since >> JVMCI is in the boot layer, the service can only be implemented by a >> provider in the boot layer and the package defining the service must be >> exported to the provider's defining module. This export currently targets >> [`jdk.internal.vm.compiler`](https://github.com/openjdk/jdk/blob/56aa1e8dc8047cbc29d554889c64beb6eca0b8eb/src/jdk.internal.vm.ci/share/classes/module-info.java#L28) >> and so the binding fails for the new Graal module. To address this, this PR >> reflects the Graal module ren aming, including adjusting the qualified export. >> >> Edit: The target of the rename is now `jdk.graal.compiler` - see >> https://github.com/openjdk/jdk/pull/16189#issuecomment-1773764186 > > Doug Simon has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Merge tag 'jdk-22+18' into JDK-8318027_rename > >Added tag jdk-22+18 for changeset 3105538d > - rename jdk.compiler.graal to jdk.graal.compiler > - re-fix since tags to reflect current JDK release > - fix copyright dates and @since tags to reflect history > - rename jdk.internal.vm.compiler* to jdk.compiler.graal* Marked as reviewed by mli (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16189#pullrequestreview-1691416502
Re: RFR: 8316645: RISC-V: Remove dependency on libatomic by adding cmpxchg 1b
On Tue, 26 Sep 2023 12:04:49 GMT, Robbin Ehn wrote: > Hi all, please consider. > > latomic is used for non native atomic operation which causes problems with > extra dependency. > This have been fixed in recent gcc, so latomic is no longer needed. > > Added new gtest, passes t1 on vf2/qemu. Nice! - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15919#pullrequestreview-1651128531
Re: RFR: 8313592: RISC-V: Link libatomic statically
On Wed, 2 Aug 2023 06:55:40 GMT, Ludovic Henry wrote: > Currently, RISC-V differs from other platforms in that it requires the > linkage to libatomic.so to support sub-word atomic operations. However, > because it is linked dynamically, it will depend on the installation of > libatomic.so on the system where the Java application will run, which no > other platform require. > > Instead of dynamically linking, we can statically link so that there is no > dependency at run-time as it's already statically linked into libjvm.so. I think you're good to go, as the cross compilation issue should not be introduced by your change, and I saw other PRs have the similar issue. - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15119#pullrequestreview-1560797009
