Skip to content

GH-46172: [C++] Add SIMD support in Meson configuration #46173

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.

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
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WillAyd
Copy link
Contributor

@WillAyd WillAyd commented Apr 16, 2025

Rationale for this change

This gets the Meson configuration closer to feature parity with CMake

What changes are included in this PR?

Adds SIMD support to the top level arrow library

Are these changes tested?

Yes

Are there any user-facing changes?

No

Copy link

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

@WillAyd
Copy link
Contributor Author

WillAyd commented Apr 16, 2025

Note that there are also SIMD modules to be used in the compute and acero directories, but those are not in main yet

I noticed that the CSV module also uses some macros around SSE4 and Neon detection, but does not use the separate module approach that is used here (see arrow/csv/lexing_internal.h). I have not looked into that too deeply; it may require a refactor to work nicely with Meson's SIMD assumptions

@WillAyd
Copy link
Contributor Author

WillAyd commented Apr 22, 2025

@github-actions crossbow submit *meson

Copy link

Revision: 607154b

Submitted crossbow builds: ursacomputing/crossbow @ actions-830ec7104c

Task Status
test-conda-cpp-meson GitHub Actions

@@ -227,6 +230,26 @@ arrow_testing_srcs = [
'testing/util.cc',
]

simd = import('unstable-simd')
rval = simd.check(
Copy link
Member

Choose a reason for hiding this comment

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

Can we use better variable name than rval?

arrow_simd_defines = []

if simd_configuration.has('HAVE_AVX2')
arrow_simd_defines += ['-DARROW_HAVE_RUNTIME_AVX2']
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to define ARROW_HAVE_AVX2 ,ARROW_HAVE_SSE4_2 and so on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah my mistake - I didn't realize we had both ARROW_HAVE_AVX2 and ARROW_HAVE_RUNTIME_AVX2; I am pretty new to SIMD so that distinction isn't yet clear to me, but yes we can go ahead and add.

It's also worth noting that the Meson SIMD check adds defines like HAVE_AVX2 through this object when that instruction set is supported. Not something we would change the mainline code for today, but in the long run that might help us simplify the macro usage

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Apr 23, 2025
@WillAyd WillAyd force-pushed the meson-simd-support branch from 607154b to 32159dd Compare April 23, 2025 12:43
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Apr 23, 2025
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