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
On Mon, 24 Jun 2024 13:30:35 GMT, Hamlin Li 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 @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! - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2195728763
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v3]
> [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 - Changes: - all: https://git.openjdk.org/jdk/pull/19185/files - new: https://git.openjdk.org/jdk/pull/19185/files/b29de559..8a08ffa1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19185=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19185=01-02 Stats: 10 lines in 1 file changed: 9 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19185.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19185/head:pull/19185 PR: https://git.openjdk.org/jdk/pull/19185
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Thu, 23 May 2024 12:49:42 GMT, Hamlin Li wrote: >> 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. 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. - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611964018
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:39:43 GMT, Hamlin Li wrote: >> make/devkit/createSleef.sh line 32: >> >>> 30: # >>> 31: # 1. cd >>> 32: # 2. bash /make/devkit/createSleef.sh aarch64-gcc.cmake >>> /devkit >> >> So you'd need a different copy of sleef for each platform? The files you >> have put in `linux/native/libvectormath`, what platform are they for? Should >> we not put them in a platform-specific subdirectory? > >> 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? - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611448592
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 [v2]
On Thu, 27 Jun 2024 21:56:19 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 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 three additional > commits since the last revision: > > - Add SLEEF version 3.6.1 > - Merge branch 'master' into 8329816-sleef > - 8329816: Add SLEEF version 3.6 make/devkit/createSleef.sh line 32: > 30: # > 31: # 1. cd > 32: # 2. bash /make/devkit/createSleef.sh aarch64-gcc.cmake > /devkit So you'd need a different copy of sleef for each platform? The files you have put in `linux/native/libvectormath`, what platform are they for? Should we not put them in a platform-specific subdirectory? - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1609623756
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, 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. I understand that you want to avoid renaming files, if they are imported. That is a good point. Moving them to an arch subdirectory does not seem like much additional hassle (there's apparently still a lot of manual work involved in upgrading the source from the third party upstream), and might help readers that are not deeply familiar with these platforms. But then again, if we only talk about header files, it is not strictly needed, so if you don't want to do it, then skip it. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2128971931
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. 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. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2128767164
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
> [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 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 three additional commits since the last revision: - Add SLEEF version 3.6.1 - Merge branch 'master' into 8329816-sleef - 8329816: Add SLEEF version 3.6 - Changes: - all: https://git.openjdk.org/jdk/pull/19185/files - new: https://git.openjdk.org/jdk/pull/19185/files/b02efa6b..b29de559 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19185=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19185=00-01 Stats: 160121 lines in 2836 files changed: 111602 ins; 34872 del; 13647 mod Patch: https://git.openjdk.org/jdk/pull/19185.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19185/head:pull/19185 PR: https://git.openjdk.org/jdk/pull/19185
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
