GH-47784: [C++] Patch vendored pcg library to enable msvc arm64 intrinsics

[jgiannuzzi]

Rationale for this change

This change enables building Arrow C++ for Windows ARM64 with MSVC when setting ARROW_SIMD_LEVEL to NONE. This is useful to be able to build Arrow with vcpkg and use it as a dependency.

Setting ARROW_SIMD_LEVEL to NONE is done to disable the use of xsimd, which does not yet support msvc arm64 intrinsics, and is non-trivial to fix.

What changes are included in this PR?

A patch to the vendored pcg library, based on imneme/pcg-cpp#99. The upstream pcg library has not been updated in the past 3 years, so this may never get merged.

Are these changes tested?

Yes, the changes have been tested in microsoft/vcpkg#47750 (the same patch for vcpkg, which this change would alleviate) and in https://github.com/jgiannuzzi/ParquetSharp/actions/runs/18354286294 (a full run of the ParquetSharp CI, using this patch to build Arrow with vcpkg).

Are there any user-facing changes?

Not really, unless you consider adding the ability to build Arrow on Windows ARM64 user-facing?

• GitHub Issue: [C++] Make Arrow C++ buildable with MSVC on Windows ARM64 #47784

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[github-actions]

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

[kou]

Hmm. It seems that https://github.com/imneme/pcg-cpp isn't maintained...

Do we have alternative implementation...?

https://github.com/imneme/pcg-cpp was vendored by #8879. @pitrou Do you have any opinion?

[pitrou]

Setting ARROW_SIMD_LEVEL to NONE is done to disable the use of xsimd, which does not yet support msvc arm64 intrinsics, and is non-trivial to fix.

@JohanMabille @AntoinePrv @serge-sans-paille Do you think xsimd can support ARM64 intrinsics on MSVC?

[pitrou]

A patch to the vendored pcg library, based on imneme/pcg-cpp#99. The upstream pcg library has not been updated in the past 3 years, so this may never get merged.

What does this patch have to do with xsimd? Am I missing something?

[jgiannuzzi]

What does this patch have to do with xsimd? Am I missing something?

Sorry @pitrou, my description wasn't clear. This patch has nothing to do with xsimd. I just meant to say that this pcg patch is not enough to build arrow with msvc on arm64 - you also need to set ARROW_SIMD_LEVEL to NONE to switch off ARROW_HAVE_NEON, which in turn will disable the use of xsimd, as it doesn't build with msvc for arm64.

[jgiannuzzi]

@JohanMabille @AntoinePrv @serge-sans-paille Do you think xsimd can support ARM64 intrinsics on MSVC?

I found this PR created in 2021 attempting to solve this: xtensor-stack/xsimd#612.

[pitrou]

Hmm. It seems that https://github.com/imneme/pcg-cpp isn't maintained...

Do we have alternative implementation...?

There is a fork here by @brt-v: https://github.com/brt-v/pcg-cpp?tab=readme-ov-file#about-this-fork
I'm not sure how reasonable it is to rely on it, though.

[brt-v]

Hmm. It seems that https://github.com/imneme/pcg-cpp isn't maintained...
Do we have alternative implementation...?

There is a fork here by @brt-v: https://github.com/brt-v/pcg-cpp?tab=readme-ov-file#about-this-fork I'm not sure how reasonable it is to rely on it, though.

I made that fork because of the lack of maintenance of the original implementation, with mostly quality of life improvements. The logic and tests are identical to the original, excepts for some small fixes to get all tests working on Windows. Note that I'm not an rng researcher and have no intention in further developing the algorithm, I just wanted to provide an easy to use library. I understand to you may not want to rely on such a small/unknown repo, but I'm happy to address your concerns and make further improvements where needed.

[JohanMabille]

Setting ARROW_SIMD_LEVEL to NONE is done to disable the use of xsimd, which does not yet support msvc arm64 intrinsics, and is non-trivial to fix.

@JohanMabille @AntoinePrv @serge-sans-paille Do you think xsimd can support ARM64 intrinsics on MSVC?

Yes it could, but I cannot give a deadline for that. For the record, an issue for tracking this feature request has already been opened.

[pitrou]

I understand to you may not want to rely on such a small/unknown repo, but I'm happy to address your concerns and make further improvements where needed.

Well, I think we could at some point. For now, we still support C++11 so we'll have to patch our vendored copy instead.

[kou]

we still support C++11

Really? I think that we require C++17:

arrow/cpp/cmake_modules/SetupCxxFlags.cmake

Lines 142 to 147 in 81ef967

	# This ensures that things like c++17 get passed correctly
	if(NOT DEFINED CMAKE_CXX_STANDARD)
	set(CMAKE_CXX_STANDARD 17)
	elseif(${CMAKE_CXX_STANDARD} VERSION_LESS 17)
	message(FATAL_ERROR "Cannot set a CMAKE_CXX_STANDARD smaller than 17")
	endif()

[pitrou]

Really? I think that we require C++17:

Wow. I even did the change myself. Clearly I haven't drunk enough tea this morning...

(I was thinking of C++20 for some reason)

[jgiannuzzi]

#47811 adds a CI job to build the C++ library with msvc arm64. I left it as draft as it depends on this current PR to be merged first. @pitrou what are your latest thoughts on it?

[pitrou]

Please update the README to include the patch that was applied, otherwise LGTM.

[pitrou]

@github-actions crossbow submit -g cpp win

[github-actions]

Revision: d6c980c

Submitted crossbow builds: ursacomputing/crossbow @ actions-c9b3535f37

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-build-cpp-fuzz
test-build-vcpkg-win
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-fedora-42-cpp
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-bundled
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer
verify-rc-source-windows
wheel-windows-cp310-cp310-amd64
wheel-windows-cp311-cp311-amd64
wheel-windows-cp312-cp312-amd64
wheel-windows-cp313-cp313-amd64
wheel-windows-cp313-cp313t-amd64
wheel-windows-cp314-cp314-amd64
wheel-windows-cp314-cp314t-amd64

[jgiannuzzi]

Please update the README to include the patch that was applied, otherwise LGTM.

Done! Could you please review again @pitrou?

[pitrou]

CI failures are unrelated, I'll merge. Thank you @jgiannuzzi !

[jgiannuzzi]

Thanks @pitrou!

[conbench-apache-arrow]

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

There were no benchmark performance regressions. 🎉

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