diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index e792574a183..2acd69ea18b 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -346,21 +346,42 @@ endmacro() macro(append_runtime_sve128_src SRCS SRC) if(ARROW_HAVE_RUNTIME_SVE128) list(APPEND ${SRCS} ${SRC}) - set_source_files_properties(${SRC} PROPERTIES COMPILE_OPTIONS "${ARROW_SVE128_FLAGS}") + set(_flags ${ARROW_SVE128_FLAGS}) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # Disable LTO to work around GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121412 + list(APPEND ${_flags} "-fno-lto") + endif() + set_property(SOURCE ${SRC} + APPEND + PROPERTY COMPILE_OPTIONS ${_flags}) endif() endmacro() macro(append_runtime_sve256_src SRCS SRC) if(ARROW_HAVE_RUNTIME_SVE256) list(APPEND ${SRCS} ${SRC}) - set_source_files_properties(${SRC} PROPERTIES COMPILE_OPTIONS "${ARROW_SVE256_FLAGS}") + set(_flags ${ARROW_SVE256_FLAGS}) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # Disable LTO to work around GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121412 + list(APPEND ${_flags} "-fno-lto") + endif() + set_property(SOURCE ${SRC} + APPEND + PROPERTY COMPILE_OPTIONS ${_flags}) endif() endmacro() macro(append_runtime_sve512_src SRCS SRC) if(ARROW_HAVE_RUNTIME_SVE512) list(APPEND ${SRCS} ${SRC}) - set_source_files_properties(${SRC} PROPERTIES COMPILE_OPTIONS "${ARROW_SVE512_FLAGS}") + set(_flags ${ARROW_SVE512_FLAGS}) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # Disable LTO to work around GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121412 + list(APPEND ${_flags} "-fno-lto") + endif() + set_property(SOURCE ${SRC} + APPEND + PROPERTY COMPILE_OPTIONS ${_flags}) endif() endmacro() @@ -588,7 +609,6 @@ append_runtime_avx2_src(ARROW_UTIL_SRCS util/byte_stream_split_internal_avx2.cc) append_runtime_avx2_src(ARROW_UTIL_SRCS util/bpacking_simd_256.cc) append_runtime_avx512_src(ARROW_UTIL_SRCS util/bpacking_simd_avx512.cc) -append_runtime_sve128_src(ARROW_UTIL_SRCS util/bpacking_simd_128_alt.cc) append_runtime_sve256_src(ARROW_UTIL_SRCS util/bpacking_simd_256.cc) if(ARROW_WITH_BROTLI) diff --git a/cpp/src/arrow/util/bpacking.cc b/cpp/src/arrow/util/bpacking.cc index 1bf81df4f28..b2992734569 100644 --- a/cpp/src/arrow/util/bpacking.cc +++ b/cpp/src/arrow/util/bpacking.cc @@ -32,9 +32,10 @@ struct UnpackDynamicFunction { static constexpr auto targets() { return std::array{ - ARROW_DISPATCH_TARGET_NONE(&bpacking::unpack_scalar) // - ARROW_DISPATCH_TARGET_NEON(&bpacking::unpack_neon) // - ARROW_DISPATCH_TARGET_SVE128(&bpacking::unpack_sve128) // + ARROW_DISPATCH_TARGET_NONE(&bpacking::unpack_scalar) // + ARROW_DISPATCH_TARGET_NEON(&bpacking::unpack_neon) // + // GH-50503: No SVE128 dispatch as it increases code size without + // increasing performance vs. Neon, and can produce ODR violations. ARROW_DISPATCH_TARGET_SVE256(&bpacking::unpack_sve256) // ARROW_DISPATCH_TARGET_SSE4_2(&bpacking::unpack_sse4_2) // ARROW_DISPATCH_TARGET_AVX2(&bpacking::unpack_avx2) // diff --git a/cpp/src/arrow/util/bpacking_benchmark.cc b/cpp/src/arrow/util/bpacking_benchmark.cc index 93d7cdf165c..025b493c592 100644 --- a/cpp/src/arrow/util/bpacking_benchmark.cc +++ b/cpp/src/arrow/util/bpacking_benchmark.cc @@ -206,11 +206,6 @@ BENCHMARK_UNPACK_ALL_TYPES_RUNTIME(Avx512Unaligned, false, bpacking::unpack_avx5 BENCHMARK_UNPACK_ALL_TYPES(NeonUnaligned, false, bpacking::unpack_neon); #endif -#if defined(ARROW_HAVE_RUNTIME_SVE128) -BENCHMARK_UNPACK_ALL_TYPES_RUNTIME(Sve128Unaligned, false, bpacking::unpack_sve128, - SVE128, "Sve128 not available"); -#endif - #if defined(ARROW_HAVE_RUNTIME_SVE256) BENCHMARK_UNPACK_ALL_TYPES_RUNTIME(Sve256Unaligned, false, bpacking::unpack_sve256, SVE256, "Sve256 not available"); diff --git a/cpp/src/arrow/util/bpacking_simd_128_alt.cc b/cpp/src/arrow/util/bpacking_simd_128_alt.cc deleted file mode 100644 index bd4799d3cd3..00000000000 --- a/cpp/src/arrow/util/bpacking_simd_128_alt.cc +++ /dev/null @@ -1,51 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#if defined(ARROW_HAVE_RUNTIME_SVE128) -# define UNPACK_PLATFORM unpack_sve128 -# define KERNEL_PLATFORM KernelSve128 -#endif - -#if !defined(UNPACK_PLATFORM) -# error "This file must be compiled with a known SIMD micro architecture" -#endif - -#include - -#include "arrow/util/bpacking_dispatch_internal.h" -#include "arrow/util/bpacking_simd_internal.h" -#include "arrow/util/bpacking_simd_kernel_internal.h" - -namespace arrow::internal::bpacking { - -template -using KERNEL_PLATFORM = Kernel>; - -template -void UNPACK_PLATFORM(const uint8_t* in, Uint* out, const UnpackOptions& opts) { - return unpack_jump(in, out, opts); -} - -template void UNPACK_PLATFORM(const uint8_t*, bool*, const UnpackOptions&); -template void UNPACK_PLATFORM(const uint8_t*, uint8_t*, const UnpackOptions&); -template void UNPACK_PLATFORM(const uint8_t*, uint16_t*, const UnpackOptions&); -template void UNPACK_PLATFORM(const uint8_t*, uint32_t*, const UnpackOptions&); -template void UNPACK_PLATFORM(const uint8_t*, uint64_t*, const UnpackOptions&); - -} // namespace arrow::internal::bpacking - -#undef UNPACK_PLATFORM diff --git a/cpp/src/arrow/util/bpacking_simd_internal.h b/cpp/src/arrow/util/bpacking_simd_internal.h index d5a81baaec0..78aaa4a8f92 100644 --- a/cpp/src/arrow/util/bpacking_simd_internal.h +++ b/cpp/src/arrow/util/bpacking_simd_internal.h @@ -53,34 +53,6 @@ extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128( #endif // UNPACK_ARCH128 #undef UNPACK_ARCH128 -#if defined(ARROW_HAVE_RUNTIME_SVE128) -# define UNPACK_ARCH128_ALT unpack_sve128 -#endif - -#if defined(UNPACK_ARCH128_ALT) - -template -ARROW_EXPORT void UNPACK_ARCH128_ALT(const uint8_t* in, Uint* out, - const UnpackOptions& opts); - -extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT( // - const uint8_t* in, bool* out, const UnpackOptions& opts); - -extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT( - const uint8_t* in, uint8_t* out, const UnpackOptions& opts); - -extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT( - const uint8_t* in, uint16_t* out, const UnpackOptions& opts); - -extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT( - const uint8_t* in, uint32_t* out, const UnpackOptions& opts); - -extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT( - const uint8_t* in, uint64_t* out, const UnpackOptions& opts); - -#endif // UNPACK_ARCH128_ALT -#undef UNPACK_ARCH128_ALT - #if defined(ARROW_HAVE_SVE256) || defined(ARROW_HAVE_RUNTIME_SVE256) # define UNPACK_ARCH256 unpack_sve256 #elif defined(UNPACK_ARCH256) || defined(ARROW_HAVE_RUNTIME_AVX2) diff --git a/cpp/src/arrow/util/bpacking_test.cc b/cpp/src/arrow/util/bpacking_test.cc index d4d588228e7..0503a15110b 100644 --- a/cpp/src/arrow/util/bpacking_test.cc +++ b/cpp/src/arrow/util/bpacking_test.cc @@ -301,15 +301,6 @@ TYPED_TEST(TestUnpack, UnpackAvx512) { TYPED_TEST(TestUnpack, UnpackNeon) { this->TestAll(&bpacking::unpack_neon); } #endif -#if defined(ARROW_HAVE_RUNTIME_SVE128) -TYPED_TEST(TestUnpack, UnpackSve128) { - if (!CpuInfo::GetInstance()->IsSupported(CpuInfo::SVE128)) { - GTEST_SKIP() << "Test requires SVE128"; - } - this->TestAll(&bpacking::unpack_sve128); -} -#endif - #if defined(ARROW_HAVE_RUNTIME_SVE256) TYPED_TEST(TestUnpack, UnpackSve256) { if (!CpuInfo::GetInstance()->IsSupported(CpuInfo::SVE256)) {