Fix unreachable constant-mask load_masked kernels (sse2, neon)#1376
Open
DiamonDinoia wants to merge 3 commits into
Open
Fix unreachable constant-mask load_masked kernels (sse2, neon)#1376DiamonDinoia wants to merge 3 commits into
DiamonDinoia wants to merge 3 commits into
Conversation
The public dispatch in xsimd_batch.hpp calls
kernel::load_masked<A>(mem, mask, convert<T>{}, mode, A{}), but the
sse2 (integral/float/double) and neon constant-mask overloads were
declared without the convert<T> parameter, so overload resolution
always fell through to the generic common kernel, which round-trips
partial loads through a zeroed stack buffer.
Evidence the overloads were dead code: the neon body called
broadcast(T(0), A{}) with a non-deducible template parameter A and
could never have compiled; fixed to broadcast<A>(T(0), A{}).
With the fix, a 2-of-4 constant prefix mask load lowers to a single
vmovq (x86, GCC/clang, sse2 through avx512) instead of
GPR->stack->vector, and neon reaches its lane-insert kernel instead
of the generic fallback.
Store kernels already had the correct 5-parameter shape and were
unaffected.
- countr_one()/countl_one() run-count conditions also matched non-contiguous masks (e.g. 0b1011 has countr_one()==2), silently dropping selected lanes on both load and store. Use exact masks. - The integral mask()==0x1 branch had no sizeof(T)==1 case and fell off the end of the function (UB, crashed test_load_store on avx2_128 once the int8 path became reachable). Restructure as an else-chain falling through to the common kernel. - Fix mm_loadu_siN -> _mm_loadu_siN typos (these overloads were previously unreachable, so they never compiled). - Add detail::lowers_to_plain_moves(mask): true when the kernels here lower the mask to plain narrow moves (proper prefix or exact upper half). Wider archs use it to decide when to delegate.
…ove masks
- Add batch_bool_constant::is_prefix()/is_suffix(): named, unit-tested
mask-shape predicates replacing ad-hoc bit tests in kernels.
- avx_128 constant-mask load/store forwarded every mask to the runtime
vmaskmov path ('constant masks gain nothing on a single register').
Wrong for masks the sse2 kernels lower to plain narrow moves: masked
stores never store-forward on Intel and are microcoded on AMD, while
movq/movlps do forward. Measured on a 2D spread RMW loop (Meteor
Lake): the plain-move tail is ~9% faster end-to-end, -49% split-line
accesses, -41% store-forward blocks. Delegate those masks to sse2;
keep the runtime path for masks vmaskmov genuinely helps.
- This also restores the intended lowering for AVX 256-bit constant-mask
half-recursion, whose half_arch resolves to avx2_128.
DiamonDinoia
force-pushed
the
fix-constant-mask-load-dispatch
branch
from
1b5c826 to
f56f450
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The public dispatch in xsimd_batch.hpp calls
kernel::load_masked(mem, mask, convert{}, mode, A{}), but the sse2 (integral/float/double) and neon constant-mask overloads were declared without the convert parameter, so overload resolution always fell through to the generic common kernel, which round-trips partial loads through a zeroed stack buffer.