On Mon, 4 Dec 2023 08:31:17 GMT, Xiaohong Gong <xg...@openjdk.org> wrote:
>> The final thing we need to resolve properly is the SVE compiler test. >> >> @theRealAph says: >>> arm_sve.h is part of GCC. It was added to GCC in 2019. >> >> A more relevant question is what version of gcc it was added, and if that >> also implies that the compiler knows about `-march=armv8-a+sve`. If so, then >> this test could basically be framed as a gcc version check. >> >> I'm still leaning towards failing configure if the SVE code cannot be >> compiled. Under what circumstances can this test possibly fail, so >> SVE_CFLAGS would not be set? > >> The final thing we need to resolve properly is the SVE compiler test. >> >> @theRealAph says: >> >> > arm_sve.h is part of GCC. It was added to GCC in 2019. >> >> A more relevant question is what version of gcc it was added, and if that >> also implies that the compiler knows about `-march=armv8-a+sve`. If so, then >> this test could basically be framed as a gcc version check. >> >> I'm still leaning towards failing configure if the SVE code cannot be >> compiled. Under what circumstances can this test possibly fail, so >> SVE_CFLAGS would not be set? > > Yes, the SVE compiler test code could be treated as a gcc/clang version > check. `arm_sve.h` which is included in `sleef.h` and then in > `vect_math_sve.c` is the SVE ACLE (Arm C Language Extensions) header file. It > was included in gcc start from version 10 (may not be exact, but gcc 8/9 > would fail when compile c code including this header). We have to make sure > the compiler supports the SVE ACLE before using it. Here are the different > scenarios: > > 1. The SVE compiler test success, and `SVE_CFLAGS` is set to > `-march=armv8-a+sve`. All symbols in `libvmath.so` are built successfully > including NEON/SVE. Hence, the vector math operations with all kinds of > vector size on both NEON/SVE machines will be improved as expected. > 2. The SVE compiler test fail, and `SVE_CFLAGS` is null. SVE symbols in > `libvmath.so` cannot be built out. Only NEON symbols exist in `libvmath.so`. > Hence, the enhancement for vector math operations with > 128-bit vector size > on SVE machines are missing. > @XiaohongGong If we are sure that the SVE test will always succeed when > running on gcc 10 or higher, then I guess I don't really need a way to > enforce SVE support -- you'll just have to make sure you use a recent enough > gcc. > > But, then the entire test becomes a bit unnecessary. You can just replace it > with a version check on gcc, or perhaps a FLAGS_COMPILER_CHECK_ARGUMENTS on > `-march=armv8-a+sve`. Thanks for the suggestion @magicus ! Replacing with a version check for the c compiler seems fine. But I cannot see the advantange than current test. Here are the reasons: 1. `-march=armv8-a+sve` is the necessary cflag for the sve source, and only included start from some c compiler versions. The c compiler version check must happen before using it. So it should also happen in the make or configure stage? Hence, we still have to find a right place to check it (should be in `lib-sleef.m4` or otherwhere?). 2. We not only have to check the gcc version, but also have to check the clang version. Would this make the code more complex? Regarding to using `FLAGS_COMPILER_CHECK_ARGUMENTS on "-march=armv8-a+sve"`, it is not right as well. Because we have to make sure the c compiler supports SVE ACLE completely which contains the sve header `arm_sve.h`. The compiler that supports option `-march=armv8-a+sve` cannot make sure the SVE ACLE is supported as well. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1839875881
