Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 14:05:51 GMT, Magnus Ihse Bursie wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comments in build system > > make/autoconf/lib-vmath.m4 line 70: > >> 68: if test "x$SYSROOT" = "x" && >> 69:test "x${LIBSLEEF_FOUND}" = "xno"; then >> 70: PKG_CHECK_MODULES([LIBSLEEF], [sleef], [LIBSLEEF_FOUND=yes], >> [LIBSLEEF_FOUND=no]) > > Suggestion: > > PKG_CHECK_MODULES([SLEEF], [sleef], [LIBSLEEF_FOUND=yes], > [LIBSLEEF_FOUND=no]) > > > Otherwise `PKG_CHECK_MODULES` will set the variables LIBSLEEF_CFLAGS and > LIBSLEEF_LIBS. Keep using `LIBSLEEF`, as the cflags and libs are named with `LIBSLEEF_` prefix. Thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1410275161
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Tue, 28 Nov 2023 01:37:01 GMT, Xiaohong Gong wrote: >>> In fact, I am not even sure why it seems to the PR author to be a good idea >>> to let the default be dependent on the build machine at all. My personal >>> opinion is that it would be better to select either "SVE enabled" or "SVE >>> disabled" as the default, and then let the user override this on the >>> configure command line, if they target a platform with different SVE >>> availability. >> >> SVE support should be enabled regardless of the build machine. The same >> binary must run on both SVE and non-SVE machines, using SVE if it is >> advantageous. I suppose some ancient C++ compilers without SVE support still >> exist, but I see no very good reason to support them building JDK 22+. >> >> Making a configure option to disable SVE support for vector math is a >> mistake, but IMO mostly harmless because no-one will ever turn it off. > >> That's fine, but we must make sure that SVE is not used by the compiler in >> any other places. If you've already built on non-SVE and tested the result >> on both SVE and non-SVE, I'm happy. > > Agree. > > Since it contains both NEON and SVE functions in libvmath.so, we have to > disable SVE feature when building those NEON functions. We want to separate > NEON/SVE functions in two files, build them with different cflags (i.e. only > build SVE sources with `-march=armv8-a+sve`), and then link to the single > `libvmath.so`. Do we have such similar examples or functions in current jdk > make system? I'm still struggling on finding out an effective way for it. > >> SVE support should be enabled regardless of the build machine. The same >> binary must run on both SVE and non-SVE machines, using SVE if it is >> advantageous. I suppose some ancient C++ compilers without SVE support still >> exist, but I see no very good reason to support them building JDK 22+. >> >> Making a configure option to disable SVE support for vector math is a >> mistake, but IMO mostly harmless because no-one will ever turn it off. > > Yes, SVE feature is also always enabled in jdk hotspot on SVE machines. If we > add the option to give people disable SVE, it's weird that we disabling the > SVE just in libvmath.so, and enabling it in hotspot. Besides, we choose the > NEON stubs for smaller than 128-bit vector operations no matter whether the > runtime machine supports SVE or not. So performance may not be an issue. > Hence, I don't think people have reason disabling SVE features. It makes no sense to configure any of this at build time. Postpone all of the decisions to runtime. Don't touch the make system.Instead, try to open the library at runtime with `os::dll_open()`, after (or inside) `VM_Version::initialize()`. If you're not running on an SVE system, none of the SVE routines will be called, so it doesn't matter, right? - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1407511713
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 16:43:09 GMT, Andrew Haley wrote: >> Apparently the situation is this: If your build machine happens to have SVE, >> then you will get SVE support in the vmath library. The SVE support will be >> used during runtime if the machine you run on has SVE support. >> >> If your build host happens to to not have SVE, then the vmath library will >> be built without SVE support, and no matter if your runtime machine has SVE >> or not, it will not provide SVE support in the vmath library. >> >> Now, if your CI farm has an arbitrarily selection of aarch64 machines with >> and without SVE, then you have no idea what you are going to get in your >> build. >> >> We have been very careful in staying clear of this kind of "random" build >> system behavior. The system you build on should not affect the output -- at >> least, not without a chance to override the default value. >> >> In fact, I am not even sure why it seems to the PR author to be a good idea >> to let the default be dependent on the build machine at all. My personal >> opinion is that it would be better to select either "SVE enabled" or "SVE >> disabled" as the default, and then let the user override this on the >> configure command line, if they target a platform with different SVE >> availability. > >> In fact, I am not even sure why it seems to the PR author to be a good idea >> to let the default be dependent on the build machine at all. My personal >> opinion is that it would be better to select either "SVE enabled" or "SVE >> disabled" as the default, and then let the user override this on the >> configure command line, if they target a platform with different SVE >> availability. > > SVE support should be enabled regardless of the build machine. The same > binary must run on both SVE and non-SVE machines, using SVE if it is > advantageous. I suppose some ancient C++ compilers without SVE support still > exist, but I see no very good reason to support them building JDK 22+. > > Making a configure option to disable SVE support for vector math is a > mistake, but IMO mostly harmless because no-one will ever turn it off. > That's fine, but we must make sure that SVE is not used by the compiler in > any other places. If you've already built on non-SVE and tested the result on > both SVE and non-SVE, I'm happy. Agree. Since it contains both NEON and SVE functions in libvmath.so, we have to disable SVE feature when building those NEON functions. We want to separate NEON/SVE functions in two files, build them with different cflags (i.e. only build SVE sources with `-march=armv8-a+sve`), and then link to the single `libvmath.so`. Do we have such similar examples or functions in current jdk make system? I'm still struggling on finding out an effective way for it. > SVE support should be enabled regardless of the build machine. The same > binary must run on both SVE and non-SVE machines, using SVE if it is > advantageous. I suppose some ancient C++ compilers without SVE support still > exist, but I see no very good reason to support them building JDK 22+. > > Making a configure option to disable SVE support for vector math is a > mistake, but IMO mostly harmless because no-one will ever turn it off. Yes, SVE feature is also always enabled in jdk hotspot on SVE machines. If we add the option to give people disable SVE, it's weird that we disabling the SVE just in libvmath.so, and enabling it in hotspot. Besides, we choose the NEON stubs for smaller than 128-bit vector operations no matter whether the runtime machine supports SVE or not. So performance may not be an issue. Hence, I don't think people have reason disabling SVE features. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1406971807
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 15:22:32 GMT, Magnus Ihse Bursie wrote: > In fact, I am not even sure why it seems to the PR author to be a good idea > to let the default be dependent on the build machine at all. My personal > opinion is that it would be better to select either "SVE enabled" or "SVE > disabled" as the default, and then let the user override this on the > configure command line, if they target a platform with different SVE > availability. SVE support should be enabled regardless of the build machine. The same binary must run on both SVE and non-SVE machines, using SVE if it is advantageous. I suppose some ancient C++ compilers without SVE support still exist, but I see no very good reason to support them building JDK 22+. Making a configure option to disable SVE support for vector math is a mistake, but IMO mostly harmless because no-one will ever turn it off. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1406450888
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 15:11:32 GMT, Andrew Haley wrote: >> You still need to separate out the SVE detection from the libsleef code, and >> provide a way to enable/disable it from the configure command line. It is >> not okay to auto-detect if features should be turned on or off by default, >> but it should always be possible to override. > >> You still need to separate out the SVE detection from the libsleef code, and >> provide a way to enable/disable it from the configure command line. > > Why? I don't think this should be a build-time option at all, because it puts > the people who build binaries in an impossible position. Can't this all be > built unconditionally, with run-time feature detection? Apparently the situation is this: If your build machine happens to have SVE, then you will get SVE support in the vmath library. The SVE support will be used during runtime if the machine you run on has SVE support. If your build host happens to to not have SVE, then the vmath library will be built without SVE support, and no matter if your runtime machine has SVE or not, it will not provide SVE support in the vmath library. Now, if your CI farm has an arbitrarily selection of aarch64 machines with and without SVE, then you have no idea what you are going to get in your build. We have been very careful in staying clear of this kind of "random" build system behavior. The system you build on should not affect the output -- at least, not without a chance to override the default value. In fact, I am not even sure why it seems to the PR author to be a good idea to let the default be dependent on the build machine at all. My personal opinion is that it would be better to select either "SVE enabled" or "SVE disabled" as the default, and then let the user override this on the configure command line, if they target a platform with different SVE availability. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1406326711
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 14:59:23 GMT, Magnus Ihse Bursie wrote: > You still need to separate out the SVE detection from the libsleef code, and > provide a way to enable/disable it from the configure command line. Why? I don't think this should be a build-time option at all, because it puts the people who build binaries in an impossible position. Can't this all be built unconditionally, with run-time feature detection? - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1406310998
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 10:28:45 GMT, Andrew Haley wrote: >> We have to use this c-compiler option to build out the SVE ABIs (e.g. >> `svfloat32_t sinfx_u10sve(svfloat32_t input)`) in `libvmath.so`. Without >> this option, at build time, all the sve related featues like `arm_sve.h / >> __ARM_FEATURE_SVE` are missing, together with the sve symbols in >> `libvmath.so` we needed at runtime. That means at runtime, hotspot cannot >> find out the sve symbols and the vector operations will fall back to the >> default java implementation. > > That's fine, but we must make sure that SVE is not used by the compiler in > any other places. If you've already built on non-SVE and tested the result on > both SVE and non-SVE, I'm happy. You still need to separate out the SVE detection from the libsleef code, and provide a way to enable/disable it from the configure command line. It is not okay to auto-detect if features should be turned on or off by default, but it should always be possible to override. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1406291471
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Mon, 27 Nov 2023 01:06:21 GMT, Xiaohong Gong wrote: >> make/autoconf/lib-vmath.m4 line 94: >> >>> 92: # Check the ARM SVE feature >>> 93: SVE_CFLAGS="-march=armv8-a+sve" >>> 94: >> >> What's this about? We're building a standard binary, and all SVE detection >> is to be deferred to runtime. > > We have to use this c-compiler option to build out the SVE ABIs (e.g. > `svfloat32_t sinfx_u10sve(svfloat32_t input)`) in `libvmath.so`. Without this > option, at build time, all the sve related featues like `arm_sve.h / > __ARM_FEATURE_SVE` are missing, together with the sve symbols in > `libvmath.so` we needed at runtime. That means at runtime, hotspot cannot > find out the sve symbols and the vector operations will fall back to the > default java implementation. That's fine, but we must make sure that SVE is not used by the compiler in any other places. If you've already built on non-SVE and tested the result on both SVE and non-SVE, I'm happy. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1405950723
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 15:43:34 GMT, Andrew Haley wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comments in build system > > make/autoconf/lib-vmath.m4 line 94: > >> 92: # Check the ARM SVE feature >> 93: SVE_CFLAGS="-march=armv8-a+sve" >> 94: > > What's this about? We're building a standard binary, and all SVE detection is > to be deferred to runtime. We have to use this c-compiler option to build out the SVE ABIs (e.g. `svfloat32_t sinfx_u10sve(svfloat32_t input)`) in `libvmath.so`. Without this option, at build time, all the sve related featues like `arm_sve.h / __ARM_FEATURE_SVE` are missing, together with the sve symbols in `libvmath.so` we needed at runtime. That means at runtime, hotspot cannot find out the sve symbols and the vector operations will fall back to the default java implementation. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1405529521
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 14:10:02 GMT, Magnus Ihse Bursie wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comments in build system > > make/autoconf/lib-vmath.m4 line 89: > >> 87: if test "x${LIBSLEEF_FOUND}" = "xyes"; then >> 88: ENABLE_LIBSLEEF=true >> 89: LIBVMATH_LIBS="${LIBVMATH_LIBS} -lsleef" > > Remove this line. It would just add `-lsleef` twice if you go via > `PKG_CHECK_MODULES`. You need to set -lsleef at the correct places. Correct. Thanks! I will adjust all the relative names/cflags once the sve cflags is removed in the m4 file. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1405526805
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 08:57:23 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: > > Address review comments in build system src/jdk.incubator.vector/linux/native/libvmath/vect_math.c line 63: > 61: DEFINE_VECTOR_MATH_UNARY(expm1d2_u10, float64x2_t, advsimd) > 62: > 63: #ifdef __ARM_FEATURE_SVE No, we're building a standard binary that will use SVE if it's present on the machine it's running on. Such compile-time feature tests are inappropriate. You need to provide SVE entry points for the target machine, no matter how HotSpot is configured and built. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403545717
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 08:57:23 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: > > Address review comments in build system make/autoconf/lib-vmath.m4 line 94: > 92: # Check the ARM SVE feature > 93: SVE_CFLAGS="-march=armv8-a+sve" > 94: What's this about? We're building a standard binary, and all SVE detection is to be deferred to runtime. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403541835
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 08:57:23 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: > > Address review comments in build system make/autoconf/lib-vmath.m4 line 58: > 56:test -e ${with_libsleef}/include/sleef.h; then > 57: LIBSLEEF_FOUND=yes > 58: LIBVMATH_LIBS="-L${with_libsleef}/lib" Suggestion: LIBVMATH_LIBS="-L${with_libsleef}/lib -lsleef" make/autoconf/lib-vmath.m4 line 70: > 68: if test "x$SYSROOT" = "x" && > 69:test "x${LIBSLEEF_FOUND}" = "xno"; then > 70: PKG_CHECK_MODULES([LIBSLEEF], [sleef], [LIBSLEEF_FOUND=yes], > [LIBSLEEF_FOUND=no]) Suggestion: PKG_CHECK_MODULES([SLEEF], [sleef], [LIBSLEEF_FOUND=yes], [LIBSLEEF_FOUND=no]) Otherwise `PKG_CHECK_MODULES` will set the variables LIBSLEEF_CFLAGS and LIBSLEEF_LIBS. make/autoconf/lib-vmath.m4 line 74: > 72: if test "x${LIBSLEEF_FOUND}" = "xno"; then > 73: AC_CHECK_HEADERS([sleef.h], > 74: [LIBSLEEF_FOUND=yes], Suggestion: [ LIBSLEEF_FOUND=yes SLEEF_LIBS=-lsleef ], make/autoconf/lib-vmath.m4 line 89: > 87: if test "x${LIBSLEEF_FOUND}" = "xyes"; then > 88: ENABLE_LIBSLEEF=true > 89: LIBVMATH_LIBS="${LIBVMATH_LIBS} -lsleef" Remove this line. It would just add `-lsleef` twice if you go via `PKG_CHECK_MODULES`. You need to set -lsleef at the correct places. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403436252 PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403433087 PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403438730 PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403437474
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 23 Nov 2023 08:57:23 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: > > Address review comments in build system make/autoconf/spec.gmk.in line 894: > 892: ENABLE_LIBSLEEF:=@ENABLE_LIBSLEEF@ > 893: LIBVMATH_CFLAGS:=@LIBVMATH_CFLAGS@ > 894: LIBVMATH_LIBS:=@LIBVMATH_LIBS@ It's getting better, but you still need to handle the naming here. It should be SLEEF_LIBS and SLEEF_CFLAGS, named after the library you import -- not the library you build. What if some other lib wants to use libsleef? And you need to untangle the SVE flags (whatever we end doing with that) from LIBVMATH_CFLAGS. - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403423892
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
> 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: Address review comments in build system - Changes: - all: https://git.openjdk.org/jdk/pull/16234/files - new: https://git.openjdk.org/jdk/pull/16234/files/b29df846..2c3c4a64 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16234&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16234&range=02-03 Stats: 126 lines in 7 files changed: 72 ins; 22 del; 32 mod Patch: https://git.openjdk.org/jdk/pull/16234.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16234/head:pull/16234 PR: https://git.openjdk.org/jdk/pull/16234