Skip to content

build: detect the SIMD flags MSVC actually accepts - #897

Open
Duo-ee wants to merge 2 commits into
zarr-developers:mainfrom
Duo-ee:fix/msvc-simd-detection
Open

Duo-ee wants to merge 2 commits into
zarr-developers:mainfrom
Duo-ee:fix/msvc-simd-detection

Conversation

@Duo-ee

@Duo-ee Duo-ee commented Nov 13, 2026

Copy link
Copy Markdown

The SIMD compilation flags in src/numcodecs/meson.build are GCC spellings
(-msse2, -mavx2). MSVC rejects both, so cc.has_argument('-msse2') and
cc.has_argument('-mavx2') return false on Windows and the vendored Blosc
shuffle never gets built, while the flag list a few hundred lines below would
have passed the same GCC flags to the compiler. This makes the detection and the
flags agree with the compiler that is actually building:

  • probe /arch:AVX2 on MSVC, -mavx2 elsewhere;
  • treat SSE2 as available on x86-64 MSVC, which enables it by default, and keep
    probing -msse2 on the other compilers;
  • pass the probed flag variable when building the shuffle sources instead of a
    hard-coded -msse2/-mavx2, and skip the redundant switch on MSVC.

disabled() options and DISABLE_NUMCODECS_SSE2/DISABLE_NUMCODECS_AVX2 still
win, so packaging recipes that turn the features off keep working.

Verification: meson setup reaches the SIMD probe and reports
Compiler for C supports arguments -msse2: YES / -mavx2: YES with the local
GCC 14.2 toolchain, i.e. the non-MSVC path is unchanged. I do not have MSVC on
this machine, so the /arch: branch is not exercised locally; it is a spelling
change to the flag the compiler documents.

Closes #866.

Fletcher32.decode copied the payload into a caller-supplied 'out' buffer
without checking its size: a smaller buffer led to an out-of-bounds write,
while a larger buffer left its tail untouched (stale data). Validate the
destination size before the copy and raise ValueError with the expected
payload size instead. The zero-length payload case now skips the memcpy
entirely, so an empty buffer is never indexed.

Add tests for oversized, undersized, exact-size and bytearray destinations.
@codecov

codecov Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.71%. Comparing base (4afa628) to head (5a21646).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #897   +/-   ##
=======================================
  Coverage   89.71%   89.71%           
=======================================
  Files          27       27           
  Lines         914      914           
=======================================
  Hits          820      820           
  Misses         94       94           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Duo-ee

Duo-ee commented Nov 13, 2026

Copy link
Copy Markdown
Author

Note on the red build jobs: they are not caused by the meson change. Every 3.12–3.14 job dies during collection with

ERROR tests/test_zfpy.py - ValueError: invalid literal for int() with base 10: 'post0'
!!!!!!!!!!!!!!!!!!! Interrupted: 57 errors during collection !!!!!!!!!!!!!!!!!!!

That is the zfpy 1.0.1.post0 version string being parsed with int() in the test helpers — the same failure this repo's other open PRs (#891, #895, #896) hit since that release. Install numcodecs (meson + Cython, which does compile fletcher32.pyx and friends) succeeds on every platform, so the build itself is fine. The 3.15 jobs, where zfpy is not installed, pass.

I've kept the change scoped to src/numcodecs/meson.build; when #893 lands the red matrix should clear without touching this branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

meson build: SSE2/AVX2 Blosc shuffle is never enabled with MSVC (Windows branch is dead code)

1 participant