From aa8ee3e88a2f914356922a3bf35e75d3a8582e58 Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Tue, 25 Oct 2022 10:06:04 +0000 Subject: [PATCH 1/2] ARROW-18162: [C++] Add Arm SVE compiler options As xsimd only supports fixed-size SVE, we have to specify vector size explicitly on command line. And the binary can only run on hardware with that vector size. Otherwise, the code behaviour is undefined. E.g., `cmake -DARROW_SIMD_LEVEL=SVE256 ..` According macro `ARROW_HAVE_SVE256` and cmake variable are defined. We can also leverage compiler auto vectorization to generate size agnostic SVE code without specifying the vector size. E.g., `cmake -DARROW_SIMD_LEVEL=SVE ..` This PR also removes some unused Arm64 arch options. --- cpp/cmake_modules/DefineOptions.cmake | 6 +++++- cpp/cmake_modules/SetupCxxFlags.cmake | 26 +++++++++++++++++--------- cpp/src/arrow/io/memory_benchmark.cc | 4 ++-- cpp/src/arrow/util/simd.h | 4 ---- python/CMakeLists.txt | 2 +- 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index 0a0f24b47f3..24923ad2416 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -185,6 +185,10 @@ takes precedence over ccache if a storage backend is configured" ON) "AVX2" "AVX512" "NEON" + "SVE" # size agnostic SVE + "SVE128" # fixed size SVE + "SVE256" # " + "SVE512" # " "DEFAULT") define_option_string(ARROW_RUNTIME_SIMD_LEVEL @@ -205,7 +209,7 @@ takes precedence over ccache if a storage backend is configured" ON) "Arm64 arch and extensions" "armv8-a" # Default "armv8-a" - "armv8-a+crc+crypto") + "armv8-a+sve") define_option(ARROW_ALTIVEC "Build with Altivec if compiler has support" ON) diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 1abc6a5fe46..df56809cb06 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -485,9 +485,7 @@ if(ARROW_CPU_FLAG STREQUAL "ppc") endif() if(ARROW_CPU_FLAG STREQUAL "armv8") - if(ARROW_SIMD_LEVEL STREQUAL "NEON") - set(ARROW_HAVE_NEON ON) - + if(ARROW_SIMD_LEVEL MATCHES "NEON|SVE[0-9]*") if(NOT CXX_SUPPORTS_ARMV8_ARCH) message(FATAL_ERROR "Unsupported arch flag: ${ARROW_ARMV8_ARCH_FLAG}.") endif() @@ -496,14 +494,24 @@ if(ARROW_CPU_FLAG STREQUAL "armv8") endif() set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} ${ARROW_ARMV8_ARCH_FLAG}") + set(ARROW_HAVE_NEON ON) add_definitions(-DARROW_HAVE_NEON) - if(ARROW_ARMV8_ARCH_FLAG MATCHES "\\+crypto") - add_definitions(-DARROW_HAVE_ARMV8_CRYPTO) - endif() - # armv8.1+ implies crc support - if(ARROW_ARMV8_ARCH_FLAG MATCHES "armv8\\.[1-9]|\\+crc") - add_definitions(-DARROW_HAVE_ARMV8_CRC) + if(ARROW_SIMD_LEVEL MATCHES "SVE[0-9]*") + if(NOT ARROW_ARMV8_ARCH_FLAG MATCHES "\\+sve") + message(FATAL_ERROR "SIMD Level is set to ${ARROW_SIMD_LEVEL} but SVE is not enabled in compiler options. " + "Add \"-DARROW_ARMV8_ARCH=armv8-a+sve\" to cmake command line to enable SVE." + ) + endif() + string(REGEX MATCH "[0-9]+" SVE_VECTOR_BITS ${ARROW_SIMD_LEVEL}) + if(SVE_VECTOR_BITS) + set(ARROW_HAVE_SVE${SVE_VECTOR_BITS} ON) + add_definitions(-DARROW_HAVE_SVE${SVE_VECTOR_BITS}) + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -msve-vector-bits=${SVE_VECTOR_BITS}") + else() + set(ARROW_HAVE_SVE_SIZELESS ON) + add_definitions(-DARROW_HAVE_SVE_SIZELSS) + endif() endif() elseif(NOT ARROW_SIMD_LEVEL STREQUAL "NONE") message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by Arm.") diff --git a/cpp/src/arrow/io/memory_benchmark.cc b/cpp/src/arrow/io/memory_benchmark.cc index 1b584d17e08..3084b5e79aa 100644 --- a/cpp/src/arrow/io/memory_benchmark.cc +++ b/cpp/src/arrow/io/memory_benchmark.cc @@ -154,7 +154,7 @@ static void StreamReadWrite(void* src, void* dst, size_t size) { #endif // ARROW_HAVE_SSE4_2 -#ifdef ARROW_HAVE_ARMV8_CRYPTO +#ifdef ARROW_HAVE_NEON using VectorType = uint8x16_t; using VectorTypeDual = uint8x16x2_t; @@ -237,7 +237,7 @@ static void StreamReadWrite(void* src, void* dst, size_t size) { } } -#endif // ARROW_HAVE_ARMV8_CRYPTO +#endif // ARROW_HAVE_NEON static void PlatformMemcpy(void* src, void* dst, size_t size) { memcpy(src, dst, size); } diff --git a/cpp/src/arrow/util/simd.h b/cpp/src/arrow/util/simd.h index 046c74cdcce..ee9105d5f4b 100644 --- a/cpp/src/arrow/util/simd.h +++ b/cpp/src/arrow/util/simd.h @@ -41,8 +41,4 @@ #include #endif -#ifdef ARROW_HAVE_ARMV8_CRC -#include -#endif - #endif diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 279ac076ac8..26a00d09e90 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -129,7 +129,7 @@ endif() if(NOT DEFINED ARROW_ARMV8_ARCH) set(ARROW_ARMV8_ARCH "armv8-a" - CACHE STRING "Arm64 arch and extensions: armv8-a, armv8-a or armv8-a+crc+crypto") + CACHE STRING "Arm64 arch and extensions") endif() include(SetupCxxFlags) From f99b57af36e8df90adad78c0dfe00b8632cd929e Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Thu, 27 Oct 2022 07:08:21 +0000 Subject: [PATCH 2/2] Remove ARROW_ARMV8_ARCH cmake option --- cpp/cmake_modules/DefineOptions.cmake | 11 --------- cpp/cmake_modules/SetupCxxFlags.cmake | 23 +++++++------------ .../conda-recipes/arrow-cpp/build-pyarrow.sh | 5 ---- python/CMakeLists.txt | 5 ---- 4 files changed, 8 insertions(+), 36 deletions(-) diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index 24923ad2416..040a6f58296 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -200,17 +200,6 @@ takes precedence over ccache if a storage backend is configured" ON) "AVX512" "MAX") - # Arm64 architectures and extensions can lead to exploding combinations. - # So set it directly through cmake command line. - # - # If you change this, you need to change the definition in - # python/CMakeLists.txt too. - define_option_string(ARROW_ARMV8_ARCH - "Arm64 arch and extensions" - "armv8-a" # Default - "armv8-a" - "armv8-a+sve") - define_option(ARROW_ALTIVEC "Build with Altivec if compiler has support" ON) define_option(ARROW_RPATH_ORIGIN "Build Arrow libraries with RATH set to \$ORIGIN" OFF) diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index df56809cb06..bf0f7dea8cd 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -110,8 +110,8 @@ elseif(ARROW_CPU_FLAG STREQUAL "ppc") endif() elseif(ARROW_CPU_FLAG STREQUAL "armv8") # Arm64 compiler flags, gcc/clang only - set(ARROW_ARMV8_ARCH_FLAG "-march=${ARROW_ARMV8_ARCH}") - check_cxx_compiler_flag(${ARROW_ARMV8_ARCH_FLAG} CXX_SUPPORTS_ARMV8_ARCH) + set(ARROW_ARMV8_MARCH "armv8-a") + check_cxx_compiler_flag("-march=${ARROW_ARMV8_MARCH}+sve" CXX_SUPPORTS_SVE) if(ARROW_SIMD_LEVEL STREQUAL "DEFAULT") set(ARROW_SIMD_LEVEL "NEON") endif() @@ -486,33 +486,26 @@ endif() if(ARROW_CPU_FLAG STREQUAL "armv8") if(ARROW_SIMD_LEVEL MATCHES "NEON|SVE[0-9]*") - if(NOT CXX_SUPPORTS_ARMV8_ARCH) - message(FATAL_ERROR "Unsupported arch flag: ${ARROW_ARMV8_ARCH_FLAG}.") - endif() - if(ARROW_ARMV8_ARCH_FLAG MATCHES "native") - message(FATAL_ERROR "native arch not allowed, please specify arch explicitly.") - endif() - set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} ${ARROW_ARMV8_ARCH_FLAG}") - set(ARROW_HAVE_NEON ON) add_definitions(-DARROW_HAVE_NEON) - if(ARROW_SIMD_LEVEL MATCHES "SVE[0-9]*") - if(NOT ARROW_ARMV8_ARCH_FLAG MATCHES "\\+sve") - message(FATAL_ERROR "SIMD Level is set to ${ARROW_SIMD_LEVEL} but SVE is not enabled in compiler options. " - "Add \"-DARROW_ARMV8_ARCH=armv8-a+sve\" to cmake command line to enable SVE." - ) + if(NOT CXX_SUPPORTS_SVE) + message(FATAL_ERROR "SVE required but compiler doesn't support it.") endif() + # -march=armv8-a+sve + set(ARROW_ARMV8_MARCH "${ARROW_ARMV8_MARCH}+sve") string(REGEX MATCH "[0-9]+" SVE_VECTOR_BITS ${ARROW_SIMD_LEVEL}) if(SVE_VECTOR_BITS) set(ARROW_HAVE_SVE${SVE_VECTOR_BITS} ON) add_definitions(-DARROW_HAVE_SVE${SVE_VECTOR_BITS}) + # -msve-vector-bits=256 set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -msve-vector-bits=${SVE_VECTOR_BITS}") else() set(ARROW_HAVE_SVE_SIZELESS ON) add_definitions(-DARROW_HAVE_SVE_SIZELSS) endif() endif() + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -march=${ARROW_ARMV8_MARCH}") elseif(NOT ARROW_SIMD_LEVEL STREQUAL "NONE") message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by Arm.") endif() diff --git a/dev/tasks/conda-recipes/arrow-cpp/build-pyarrow.sh b/dev/tasks/conda-recipes/arrow-cpp/build-pyarrow.sh index 60144d06b9d..76fdbce0c89 100644 --- a/dev/tasks/conda-recipes/arrow-cpp/build-pyarrow.sh +++ b/dev/tasks/conda-recipes/arrow-cpp/build-pyarrow.sh @@ -30,11 +30,6 @@ else export PYARROW_WITH_CUDA=0 fi -# Resolve: Make Error at cmake_modules/SetupCxxFlags.cmake:338 (message): Unsupported arch flag: -march=. -if [[ "${target_platform}" == "linux-aarch64" ]]; then - export PYARROW_CMAKE_OPTIONS="-DARROW_ARMV8_ARCH=armv8-a ${PYARROW_CMAKE_OPTIONS}" -fi - if [[ "${target_platform}" == osx-* ]]; then # See https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk CXXFLAGS="${CXXFLAGS} -D_LIBCPP_DISABLE_AVAILABILITY" diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 26a00d09e90..bad5e926abb 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -126,11 +126,6 @@ if(NOT DEFINED ARROW_RUNTIME_SIMD_LEVEL) "MAX" CACHE STRING "Max runtime SIMD optimization level") endif() -if(NOT DEFINED ARROW_ARMV8_ARCH) - set(ARROW_ARMV8_ARCH - "armv8-a" - CACHE STRING "Arm64 arch and extensions") -endif() include(SetupCxxFlags) # Add common flags