Enable MSVC (cl.exe) build on Windows ARM64#1285
Enable MSVC (cl.exe) build on Windows ARM64#1285pdeep854 wants to merge 1 commit intoxtensor-stack:masterfrom
Conversation
serge-sans-paille
left a comment
serge-sans-paille
left a comment
There was a problem hiding this comment.
First set of review, more to come later but at least it will trigger the discussion
| #include <algorithm> | ||
| #include <array> | ||
| #include <complex> | ||
| #include <cstdio> |
There was a problem hiding this comment.
unused header, probably a leftover from a debug session.
| for (size_t i = 0; i < sizeof...(Is); ++i) | ||
| if ((bitmask >> i) & 1u) | ||
| std::swap(mask_buffer[inserted++], mask_buffer[i]); | ||
| // Fill remaining positions with the last valid index to avoid undefined behavior |
There was a problem hiding this comment.
why not always setting zero?
There was a problem hiding this comment.
after swapping some unused entries at the end of mask_buffer still contain old index values. This mask is later used by swizzle() to gather vector elements. on MSVC arm64, swizzle() returns 0 for invalid indices, so these leftover values cause incorrect results.
to fix this the unused entries are filled with the last valid index, which acts as a safe placeholder. can also set it to 0
| compress_mask.store_aligned(&mask_out[0]); | ||
| alignas(A::alignment()) T z_out[size]; | ||
| z.store_aligned(&z_out[0]); | ||
| auto res = swizzle(z, compress_mask); |
There was a problem hiding this comment.
what if the overload we were using previously actually exists?
There was a problem hiding this comment.
thanks will remove it as it was added for debugging
| alignas(16) T data[] = { static_cast<T>(args)... }; | ||
| if constexpr (sizeof(T) == 1) | ||
| { | ||
| if constexpr (std::is_unsigned<T>::value) |
There was a problem hiding this comment.
xsimd requires only C++14 as of this PR :-/
| { | ||
| #if defined(_MSC_VER) && defined(_M_ARM64) | ||
| alignas(16) double data[] = { d0, d1 }; | ||
| return vld1q_f64(data); |
There was a problem hiding this comment.
we could make this the default if it's more prtable: https://godbolt.org/z/fP3Tvjv5f
| static_cast<unsigned_type>(b0 ? -1LL : 0LL), | ||
| static_cast<unsigned_type>(b1 ? -1LL : 0LL) | ||
| }; | ||
| return vld1q_u64(data); |
2 participants
PR: Enable MSVC (cl.exe) build on Windows ARM64
Summary
This PR restores full build compatibility for xsimd with the MSVC compiler (
cl.exe) onWindows ARM64 (
_M_ARM64). Prior to this change, the project failed to compile with MSVCon ARM64 due to fundamental differences in how MSVC exposes ARM NEON intrinsics compared
to GCC and Clang.
All changes are strictly additive and backward-compatible: every existing GCC/Clang code
path is preserved unchanged inside
#elsebranches. No behaviour is altered for anynon-MSVC-ARM64 target.
Testing
The build was verified locally on a Windows ARM64 machine using Visual Studio 2022:
Result: All three targets build successfully with zero errors: