Skip to content

GH-49449: [C++] Backport xsimd neon fix#49450

Merged
pitrou merged 1 commit intoapache:mainfrom
AntoinePrv:xsimd-neon-backport
Mar 9, 2026
Merged

GH-49449: [C++] Backport xsimd neon fix#49450
pitrou merged 1 commit intoapache:mainfrom
AntoinePrv:xsimd-neon-backport

Conversation

@AntoinePrv
Copy link
Contributor

@AntoinePrv AntoinePrv commented Mar 4, 2026

Rationale for this change

Performance in hot code.
There is a bug in xsimd where the rshift/lshift for Neon are implemented with a scalar loop instead of the appropriate SIMD intrinsics. This code path is core to the unpack routine in Parquet reads.
xtensor-stack/xsimd#1266

What changes are included in this PR?

A bug backport.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions
Copy link

github-actions bot commented Mar 4, 2026

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

@AntoinePrv AntoinePrv changed the title GH-49449 GH-49449: [C++] Backport xsimd neon fix Mar 4, 2026
@github-actions github-actions bot added the awaiting review Awaiting review label Mar 4, 2026
@AntoinePrv AntoinePrv force-pushed the xsimd-neon-backport branch from 44a1a10 to 18ce22f Compare March 4, 2026 13:56
@AntoinePrv
Copy link
Contributor Author

@pitrou

@pitrou
Copy link
Member

pitrou commented Mar 4, 2026

Can you explain what this is fixing in the PR description?

@pitrou
Copy link
Member

pitrou commented Mar 4, 2026

Also, were you able to run some benchmarks?

@AntoinePrv
Copy link
Contributor Author

Also, were you able to run some benchmarks?

Not yet, I will do it overnight (I run some extensive benchmarks in a fork). I was however able to confirm from the assembly that this is the correct instructions we aimed to use.

@AntoinePrv
Copy link
Contributor Author

@pitrou I've run the benchmarks (though I have lost the old ones in the process) and it is much better.
It went from being on par with the scalar / old version to being much better.

@pitrou
Copy link
Member

pitrou commented Mar 6, 2026

@github-actions crossbow submit -g cpp

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Revision: 18ce22f

Submitted crossbow builds: ursacomputing/crossbow @ actions-471834543b

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-debian-13-cpp-amd64 GitHub Actions
test-debian-13-cpp-i386 GitHub Actions
test-debian-experimental-cpp-gcc-15 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@pitrou
Copy link
Member

pitrou commented Mar 6, 2026

@ursabot please benchmark lang=C++

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

+1, I trust you on the benchmarks given that our comment bot seem out of order (@rok). Will merge.

@pitrou pitrou merged commit dbbf7cf into apache:main Mar 9, 2026
53 of 56 checks passed
@pitrou pitrou removed the awaiting review Awaiting review label Mar 9, 2026
@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Mar 9, 2026
@rok
Copy link
Member

rok commented Mar 9, 2026

@ursabot please benchmark lang=C++

@rok
Copy link
Member

rok commented Mar 9, 2026

Benchmark runs are scheduled for commit 18ce22f. Watch https://buildkite.com/apache-arrow and https://conbench.arrow-dev.org for updates. A comment will be posted here when the runs are complete.

@rok
Copy link
Member

rok commented Mar 9, 2026

I didn't realize we already cover neon via graviton machines, that's nice.

@conbench-apache-arrow
Copy link

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

There were no benchmark performance regressions. 🎉

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

@rok
Copy link
Member

rok commented Mar 9, 2026

@pitrou benchmarks look ok.

@conbench-apache-arrow
Copy link

Thanks for your patience. Conbench analyzed the 3 benchmarking runs that have been run so far on PR commit 18ce22f.

There were 7 benchmark results indicating a performance regression:

The full Conbench report has more details.

@AntoinePrv AntoinePrv deleted the xsimd-neon-backport branch March 10, 2026 08:57
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.

3 participants