Skip to content

GH-50542: [C++] Fix ARROW_SIMD_LEVEL=NONE build, compile SSE4.2 kernels#50547

Open
domibel wants to merge 1 commit into
apache:mainfrom
domibel:build-sse4.2-kernels
Open

GH-50542: [C++] Fix ARROW_SIMD_LEVEL=NONE build, compile SSE4.2 kernels#50547
domibel wants to merge 1 commit into
apache:mainfrom
domibel:build-sse4.2-kernels

Conversation

@domibel

@domibel domibel commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

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

…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.
@domibel
domibel requested a review from pitrou as a code owner July 19, 2026 18:36
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50542 has been automatically assigned in GitHub to PR creator.

@AntoinePrv

Copy link
Copy Markdown
Collaborator

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++.
I believe the reason at the time was the unconditional usage of popcnt, but perhaps now that we moved to std::popcnt this is more portable.
There may be more places in the CMakeLists.txt where the default compiler options do pick a minimal SSE4.2 baseline so that would be a place to look at as well.

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?

@domibel

domibel commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants