Skip to content

GH-45331: [C++] Use xsimd for CPU feature detection - #49940

Merged
pitrou merged 6 commits into
apache:mainfrom
AntoinePrv:cpu-feature
Jun 10, 2026
Merged

GH-45331: [C++] Use xsimd for CPU feature detection#49940
pitrou merged 6 commits into
apache:mainfrom
AntoinePrv:cpu-feature

Conversation

@AntoinePrv

@AntoinePrvAntoinePrv commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Rationale for this change

Use xsimd cpu feature instead of maitaining them here.
Stacked on GH-49922.
Should also handle issue from GH-30368.

What changes are included in this PR?

  • Use xsimd for CPu feature detection, cache sizes are still using the same implementation
  • We are loosing CPU model name (which was unused)
  • Simplify CpuInfo (remove Pimpl pattern)

Are these changes tested?

in CI

Are there any user-facing changes?

No

@github-actions

Copy link
Copy Markdown

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

@AntoinePrv
AntoinePrvforce-pushed the cpu-feature branch 2 times, most recently from 2958760 to c132835CompareMay 7, 2026 09:12
CopilotAI review requested due to automatic review settings June 3, 2026 12:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR refactors Arrow’s CpuInfo implementation by removing the PIMPL indirection and migrating runtime CPU feature detection to xsimd.

Changes:

  • Replace OS/arch-specific CPUID and /proc/cpuinfo parsing with xsimd-based feature detection.
  • Inline CpuInfo accessors in the header and store state directly on the class (no Impl).
  • Generalize OS cache-size retrieval helpers to be std::array-size agnostic via templates.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

FileDescription
cpp/src/arrow/util/cpu_info.hInlines getters, removes PIMPL plumbing, and changes the model_name() API/behavior.
cpp/src/arrow/util/cpu_info.ccImplements xsimd-backed CPU feature detection, removes legacy per-OS detection, and stores state directly in CpuInfo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcpp/src/arrow/util/cpu_info.cc
Comment threadcpp/src/arrow/util/cpu_info.cc
Comment threadcpp/src/arrow/util/cpu_info.h
Comment threadcpp/src/arrow/util/cpu_info.cc
Comment threadcpp/src/arrow/util/cpu_info.cc Outdated
CopilotAI review requested due to automatic review settings June 3, 2026 13:50

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment threadcpp/src/arrow/util/cpu_info.h
Comment threadcpp/src/arrow/util/cpu_info.cc
Comment threadcpp/src/arrow/util/cpu_info.cc
@AntoinePrv
AntoinePrv marked this pull request as ready for review June 3, 2026 15:07
@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

@pitrou this is in pretty good shape !

@pitroupitrou added the CI: Extra: C++ Run extra C++ CI label Jun 4, 2026

@pitroupitrou left a comment

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.

Neat, thanks a lot for doing this @AntoinePrv

@pitrou

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g cpp

@pitrou

Copy link
Copy Markdown
Member

@github-actions crossbow submit wheelcp314*

@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jun 4, 2026
@github-actions

Copy link
Copy Markdown

Revision: 557d65b

Submitted crossbow builds: ursacomputing/crossbow @ actions-ee545fa2be

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-debian-13-cpp-amd64GitHub Actions
test-debian-13-cpp-i386GitHub Actions
test-debian-experimental-cpp-gcc-15GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

@github-actions

Copy link
Copy Markdown

Revision: 557d65b

Submitted crossbow builds: ursacomputing/crossbow @ actions-055eb82a87

TaskStatus
wheel-macos-monterey-cp314-cp314-amd64GitHub Actions
wheel-macos-monterey-cp314-cp314-arm64GitHub Actions
wheel-macos-monterey-cp314-cp314t-amd64GitHub Actions
wheel-macos-monterey-cp314-cp314t-arm64GitHub Actions
wheel-manylinux-2-28-cp314-cp314-amd64GitHub Actions
wheel-manylinux-2-28-cp314-cp314-arm64GitHub Actions
wheel-manylinux-2-28-cp314-cp314t-amd64GitHub Actions
wheel-manylinux-2-28-cp314-cp314t-arm64GitHub Actions
wheel-musllinux-1-2-cp314-cp314-amd64GitHub Actions
wheel-musllinux-1-2-cp314-cp314-arm64GitHub Actions
wheel-musllinux-1-2-cp314-cp314t-amd64GitHub Actions
wheel-musllinux-1-2-cp314-cp314t-arm64GitHub Actions
wheel-windows-cp314-cp314-amd64GitHub Actions
wheel-windows-cp314-cp314t-amd64GitHub Actions

@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

We have two

/arrow/cpp/src/arrow/util/cpu_info.cc:29:10: fatal error: 'xsimd/xsimd.hpp' file not found

I wonder if it is becuase previous includes were gated by
#if defined(ARROW_HAVE_NEON) || defined(ARROW_HAVE_SSE4_2)
and we are now catching cases were xsimd is in fact not available.

@pitrou

Copy link
Copy Markdown
Member

Ahah. Well, let's make it mandatory then? See

if((NOT ARROW_SIMD_LEVEL STREQUAL"NONE") OR (NOT ARROW_RUNTIME_SIMD_LEVEL STREQUAL"NONE"
))
set(ARROW_USE_XSIMD TRUE)
else()
set(ARROW_USE_XSIMD FALSE)
endif()

@AntoinePrv
AntoinePrv requested a review from wgtmac as a code ownerJune 5, 2026 09:32
@github-actionsgithub-actionsBot removed the CI: Extra: C++ Run extra C++ CI label Jun 5, 2026
@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: f5cab60

Submitted crossbow builds: ursacomputing/crossbow @ actions-f9e8dca2bd

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-debian-13-cpp-amd64GitHub Actions
test-debian-13-cpp-i386GitHub Actions
test-debian-experimental-cpp-gcc-15GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

CopilotAI review requested due to automatic review settings June 5, 2026 14:38
@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

I pushed a fix for emscripten (and other ?), though I think the issue must have been there before.
I think the rest must be unreleated.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment threadcpp/src/arrow/util/cpu_info.cc
Comment threadcpp/src/arrow/util/cpu_info.h
Comment threadcpp/src/arrow/util/cpu_info.cc
@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: feda2a5

Submitted crossbow builds: ursacomputing/crossbow @ actions-eced990723

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-debian-13-cpp-amd64GitHub Actions
test-debian-13-cpp-i386GitHub Actions
test-debian-experimental-cpp-gcc-15GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

@pitroupitrou added the CI: Extra: C++ Run extra C++ CI label Jun 9, 2026

@pitroupitrou left a comment

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.

+1, LGTM. Just one question.

Comment on lines +854 to +855
list(APPEND ARROW_COMPUTE_STATIC_LINK_LIBS ${ARROW_XSIMD})
list(APPEND ARROW_COMPUTE_SHARED_PRIVATE_LINK_LIBS ${ARROW_XSIMD})

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.

I don't see xsimd in use anywhere in arrow/compute, do you remember why this was required @raulcd ?

Edit: perhaps because of src/arrow/util/utf8_internal.h? It's a pity this indirect dependency isn't picked up automatically.

@pitroupitrou changed the title GH-45331: Use xsimd for CPU feature detectionGH-45331: [C++≠ Use xsimd for CPU feature detectionJun 9, 2026
@pitroupitrou changed the title GH-45331: [C++≠ Use xsimd for CPU feature detectionGH-45331: [C++] Use xsimd for CPU feature detectionJun 9, 2026
@pitrou

Copy link
Copy Markdown
Member

@github-actions crossbow submit wheelcp314*

@github-actions

Copy link
Copy Markdown

Revision: feda2a5

Submitted crossbow builds: ursacomputing/crossbow @ actions-f1dbe8e148

TaskStatus
wheel-macos-monterey-cp314-cp314-amd64GitHub Actions
wheel-macos-monterey-cp314-cp314-arm64GitHub Actions
wheel-macos-monterey-cp314-cp314t-amd64GitHub Actions
wheel-macos-monterey-cp314-cp314t-arm64GitHub Actions
wheel-manylinux-2-28-cp314-cp314-amd64GitHub Actions
wheel-manylinux-2-28-cp314-cp314-arm64GitHub Actions
wheel-manylinux-2-28-cp314-cp314t-amd64GitHub Actions
wheel-manylinux-2-28-cp314-cp314t-arm64GitHub Actions
wheel-musllinux-1-2-cp314-cp314-amd64GitHub Actions
wheel-musllinux-1-2-cp314-cp314-arm64GitHub Actions
wheel-musllinux-1-2-cp314-cp314t-amd64GitHub Actions
wheel-musllinux-1-2-cp314-cp314t-arm64GitHub Actions
wheel-windows-cp314-cp314-amd64GitHub Actions
wheel-windows-cp314-cp314t-amd64GitHub Actions

@pitrou
pitrou merged commit ca47cd1 into apache:mainJun 10, 2026
86 of 87 checks passed
@pitroupitrou removed the awaiting committer review Awaiting committer review label Jun 10, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit ca47cd1.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce them.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AntoinePrv@pitrou