GH-50542: [C++] Fix ARROW_SIMD_LEVEL=NONE build, compile SSE4.2 kernels#50547
GH-50542: [C++] Fix ARROW_SIMD_LEVEL=NONE build, compile SSE4.2 kernels#50547domibel wants to merge 1 commit into
Conversation
…e SSE4.2 kernels Building with ARROW_SIMD_LEVEL=NONE and ARROW_RUNTIME_SIMD_LEVEL=MAX failed with: bpacking.cc:39:49: error: 'unpack_sse4_2' is not a member of 'arrow::internal::bpacking' The SSE4.2 dispatch table entries are emitted when either the compile-time or the runtime macro is set, but the kernels themselves were only compiled when the compile-time macro was set. So I moved the byte_stream_split SSE4.2 instantiations into a dedicated translation unit, keeping the dispatcher's own unit at the baseline ISA.
|
|
|
The PR looks sensible but this may not be the only limitation. From what I understood when last working on this part of the code, SSE4.2 is mandatory when building Arrow C++. In principle I think treating all SIMD as optional makes the code more uniform, and is a free abstraction since the dispatch will change to static when possible. @domibel I'm curious what is your use case? Do you only wish to disable SIMD, or do you need to target below SSE4.2? |
|
@AntoinePrv Debian is using the x86-64-v1 baseline, so we build with ARROW_SIMD_LEVEL=NONE but keep ARROW_RUNTIME_SIMD_LEVEL=MAX for dispatching at runtime. That means we don't disable SIMD, we just keep it out of the baseline. The PR is fixing a regression introduced somewhere after the 23 release. @kou Could ARROW_SIMD_LEVEL=NONE be added to the nightly test coverage? |
Rationale for this change
Building with ARROW_SIMD_LEVEL=NONE and ARROW_RUNTIME_SIMD_LEVEL=MAX failed with:
bpacking.cc:39:49: error: 'unpack_sse4_2' is not a member of
'arrow::internal::bpacking'
The SSE4.2 dispatch table entries are emitted when either the compile-time or the runtime macro is set, but the kernels themselves were only compiled when the compile-time macro was set.
What changes are included in this PR?
I moved the byte_stream_split SSE4.2 instantiations into a dedicated translation unit, keeping the dispatcher's own unit at the baseline ISA.
Are these changes tested?
Yes
Are there any user-facing changes?
No