Skip to content

fix(neon): under-aligned store in generic swizzle kernel#1374

Merged
serge-sans-paille merged 2 commits into
xtensor-stack:masterfrom
vsaraikin:fix/neon-generic-swizzle-alignment
Jul 8, 2026
Merged

fix(neon): under-aligned store in generic swizzle kernel#1374
serge-sans-paille merged 2 commits into
xtensor-stack:masterfrom
vsaraikin:fix/neon-generic-swizzle-alignment

Conversation

@vsaraikin

Copy link
Copy Markdown
Contributor

Fixes #1260.

The generic neon swizzle kernel stores the batch into a local std::array<T, size>, which is only aligned to alignof(T) — 1 byte for the 8-bit batches that take this path (armv7 neon has no vqtbl1q; 8/16-bit swizzles fall through to the generic kernel). store_aligned requires the full batch alignment, so whenever the compiler places the array at a non-16-byte-aligned stack offset the alignment assertion fires: reduce_max → kernel::swizzle(neon) → store_aligned abort — exactly the Debian armhf backtrace in the issue.

This is not armhf-specific: the same abort reproduces on arm64 macOS with batch<unsigned char, xsimd::neon> compiled at -O2 with assertions enabled:

using batch_type = xsimd::batch<unsigned char, xsimd::neon>;
alignas(16) unsigned char raw[16] = { 0, 1, 2, 3, /**/ };
auto b = batch_type::load_aligned(raw);
xsimd::reduce_max(b); // Assertion failed: store location is not properly aligned

The fix declares the buffer alignas(A::alignment()), matching the existing idiom in xsimd_neon64.hpp (alignas(A::alignment()) double data[] = …).

Adds a NEON-guarded regression test exercising the generic swizzle kernel and reduce_max for all four 8/16-bit batch types on the neon arch. One honest caveat: whether the assertion fires on the old code depends on the frame layout the compiler happens to pick (it fires in the issue's Debian build and in the -O2 repro above, but not in a local -O0 doctest build), so the test documents and exercises the path rather than deterministically aborting on regression.

Full test suite passes on arm64 macOS (391 test cases / 5928 assertions).

Comment thread test/test_batch_manip.cpp Outdated
SUBCASE("rotate_right") { swizzle_test<B>().rotate_right(); }
}

#if XSIMD_WITH_NEON

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no need to run this only in the neon case you can run this as a unit test on all architectures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — dropped the NEON guard; the test now iterates xsimd::supported_architectures (gated on Arch::available()), so the narrow swizzle kernels and reduce_max run for every enabled arch on each CI target.

@serge-sans-paille

Copy link
Copy Markdown
Contributor

LGTM once @DiamonDinoia comment has been addressed.

@vsaraikin vsaraikin marked this pull request as ready for review July 8, 2026 10:03
@serge-sans-paille serge-sans-paille merged commit 7778faa into xtensor-stack:master Jul 8, 2026
89 checks passed
@serge-sans-paille

Copy link
Copy Markdown
Contributor

Thanks 🙇

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.

SIGABRT on 32bit armhf in [batch]<xsimd::batch<unsigned char>>

3 participants