On Thu, 14 Mar 2024 09:14:04 GMT, Hamlin Li <m...@openjdk.org> 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 variable name in github workflow make/autoconf/flags-cflags.m4 line 994: > 992: # for SVE. Set SVE_CFLAGS to -march=armv8-a+sve if it does. Empty > otherwise. > 993: # ACLE and this flag are required to build the Arm SVE related > functions in > 994: # libvmath. Suggestion: # Check whether the compiler supports the Arm C Language Extensions (ACLE) # for SVE. Set SVE_CFLAGS to -march=armv8-a+sve if it does. # ACLE and this flag are required to build the aarch64 SVE related functions in # libvectormath. 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.) src/jdk.incubator.vector/linux/native/libvmath/vect_math.h line 31: > 29: #endif > 30: > 31: #ifndef VMATH_EXPORT You can just use `JNIEXPORT` instead of trying to brew your own solution. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1524748090 PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1524745947 PR Review Comment: https://git.openjdk.org/jdk/pull/18294#discussion_r1524746666
