Skip to content

GH-50503: [Parquet] Remove SVE128 unpack #50611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AntoinePrv wants to merge 6 commits into
base: main
Choose a base branch
from
Open

GH-50503: [Parquet] Remove SVE128 unpack #50611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a6e5065
Remove SVE128 path
AntoinePrv Jul 23, 2026
3458a70
Run CI
AntoinePrv Jul 23, 2026
e84d38f
Rerun
AntoinePrv Jul 24, 2026
82cd6a0
Add no-lto to SVE dispatch
AntoinePrv Jul 28, 2026
8ee72b9
Disable LTO only on GCC
pitrou Jul 28, 2026
504bd8e
Add comment
pitrou Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions cpp/src/arrow/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})

@pitrou pitrou Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kou Do these CMake changes look ok?

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()

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/arrow/util/bpacking.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ struct UnpackDynamicFunction {

static constexpr auto targets() {
return std::array{
ARROW_DISPATCH_TARGET_NONE(&bpacking::unpack_scalar<Uint>) //
ARROW_DISPATCH_TARGET_NEON(&bpacking::unpack_neon<Uint>) //
ARROW_DISPATCH_TARGET_SVE128(&bpacking::unpack_sve128<Uint>) //
Comment thread
pitrou marked this conversation as resolved.
ARROW_DISPATCH_TARGET_NONE(&bpacking::unpack_scalar<Uint>) //
ARROW_DISPATCH_TARGET_NEON(&bpacking::unpack_neon<Uint>) //
// 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<Uint>) //
ARROW_DISPATCH_TARGET_SSE4_2(&bpacking::unpack_sse4_2<Uint>) //
ARROW_DISPATCH_TARGET_AVX2(&bpacking::unpack_avx2<Uint>) //
Expand Down
5 changes: 0 additions & 5 deletions cpp/src/arrow/util/bpacking_benchmark.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
51 changes: 0 additions & 51 deletions cpp/src/arrow/util/bpacking_simd_128_alt.cc
View file
Open in desktop

This file was deleted.

28 changes: 0 additions & 28 deletions cpp/src/arrow/util/bpacking_simd_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,34 +53,6 @@ extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128<uint64_t>(
#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 <typename Uint>
ARROW_EXPORT void UNPACK_ARCH128_ALT(const uint8_t* in, Uint* out,
const UnpackOptions& opts);

extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT<bool>( //
const uint8_t* in, bool* out, const UnpackOptions& opts);

extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT<uint8_t>(
const uint8_t* in, uint8_t* out, const UnpackOptions& opts);

extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT<uint16_t>(
const uint8_t* in, uint16_t* out, const UnpackOptions& opts);

extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT<uint32_t>(
const uint8_t* in, uint32_t* out, const UnpackOptions& opts);

extern template ARROW_TEMPLATE_EXPORT void UNPACK_ARCH128_ALT<uint64_t>(
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)
Expand Down
9 changes: 0 additions & 9 deletions cpp/src/arrow/util/bpacking_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,6 @@ TYPED_TEST(TestUnpack, UnpackAvx512) {
TYPED_TEST(TestUnpack, UnpackNeon) { this->TestAll(&bpacking::unpack_neon<TypeParam>); }
#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<TypeParam>);
}
#endif

#if defined(ARROW_HAVE_RUNTIME_SVE256)
TYPED_TEST(TestUnpack, UnpackSve256) {
if (!CpuInfo::GetInstance()->IsSupported(CpuInfo::SVE256)) {
Expand Down
Loading