Skip to content

Commit 4bb4fe4

Browse files
committed
fix: correct constant-mask load/store kernels and add shape predicates
The sse2 and neon constant-mask load_masked kernels were unreachable: they lacked the convert<T> overload parameter the dispatcher passes, so calls fell through to the runtime-mask path. Add the parameter and fix the mask conditions, which used ambiguous count-based tests (e.g. countr_one()==2 also matches 0b1011, silently dropping a lane). Add batch_bool_constant::is_prefix/is_suffix shape predicates plus prefix()/ suffix() returning the set-run length (or size+1 when the mask is not that shape, keeping == exact). The sse2 kernels now dispatch on these instead of raw bit patterns, so each condition reads as the lane shape it handles. avx_128 delegates prefix/upper-half masks to sse2: those lower to plain moves (vmovss/movlps/…) which store-forward, whereas vmaskmov never does on Intel and its stores are microcoded on AMD. Tests cover the shape predicates including off-by-one boundaries (size-1 vs the size+1 sentinel) and near-full mask patterns through the load/store kernels.
1 parent 6488be7 commit 4bb4fe4

6 files changed

Lines changed: 195 additions & 49 deletions

File tree

include/xsimd/arch/xsimd_avx_128.hpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,19 @@ namespace xsimd
103103
return _mm_cmp_pd(self, other, _CMP_NEQ_UQ);
104104
}
105105

106-
// constant masks gain nothing on a single register; forward to runtime
106+
// Masks that lower to plain moves go to sse2; the rest gain nothing on a
107+
// single register, so take the runtime path.
107108
template <class A, class T, bool... Values, class Mode, class = std::enable_if_t<std::is_floating_point<T>::value>>
108109
XSIMD_INLINE batch<T, A> load_masked(T const* mem, batch_bool_constant<T, A, Values...> mask, convert<T>, Mode, requires_arch<avx_128>) noexcept
109110
{
110-
return load_masked(mem, mask.as_batch_bool(), convert<T> {}, Mode {}, avx_128 {});
111+
XSIMD_IF_CONSTEXPR(detail::lowers_to_plain_moves(mask))
112+
{
113+
return load_masked(mem, mask, convert<T> {}, Mode {}, sse2 {});
114+
}
115+
else
116+
{
117+
return load_masked(mem, mask.as_batch_bool(), convert<T> {}, Mode {}, avx_128 {});
118+
}
111119
}
112120

113121
// Runtime-mask load (float/double).
@@ -142,7 +150,14 @@ namespace xsimd
142150
template <class A, class T, bool... Values, class Mode, class = std::enable_if_t<std::is_floating_point<T>::value>>
143151
XSIMD_INLINE void store_masked(T* mem, batch<T, A> const& src, batch_bool_constant<T, A, Values...> mask, Mode, requires_arch<avx_128>) noexcept
144152
{
145-
store_masked(mem, src, mask.as_batch_bool(), Mode {}, avx_128 {});
153+
XSIMD_IF_CONSTEXPR(detail::lowers_to_plain_moves(mask))
154+
{
155+
store_masked(mem, src, mask, Mode {}, sse2 {});
156+
}
157+
else
158+
{
159+
store_masked(mem, src, mask.as_batch_bool(), Mode {}, avx_128 {});
160+
}
146161
}
147162

148163
// Runtime-mask store (float/double).

include/xsimd/arch/xsimd_neon.hpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,8 @@ namespace xsimd
539539
template <>
540540
struct load_masked<>
541541
{
542-
template <size_t I, class A, class T, bool Use>
543-
static XSIMD_INLINE batch<T, A> apply(T const* /* mem */, batch<T, A> acc, std::integral_constant<bool, Use>) noexcept
542+
template <size_t I, class A, class T>
543+
static XSIMD_INLINE batch<T, A> apply(T const* /* mem */, batch<T, A> acc) noexcept
544544
{
545545
return acc;
546546
}
@@ -549,23 +549,21 @@ namespace xsimd
549549
struct load_masked<Value, Values...>
550550
{
551551
template <size_t I, class A, class T>
552-
static XSIMD_INLINE batch<T, A> apply(T const* mem, batch<T, A> acc, std::true_type) noexcept
553-
{
554-
return load_masked<Values...>::template apply<I + 1>(mem, insert(acc, mem[I], index<I> {}), std::integral_constant<bool, Value> {});
555-
}
556-
template <size_t I, class A, class T>
557-
static XSIMD_INLINE batch<T, A> apply(T const* mem, batch<T, A> acc, std::false_type) noexcept
552+
static XSIMD_INLINE batch<T, A> apply(T const* mem, batch<T, A> acc) noexcept
558553
{
559-
return load_masked<Values...>::template apply<I + 1>(mem, acc, std::integral_constant<bool, Value> {});
554+
XSIMD_IF_CONSTEXPR(Value)
555+
{
556+
acc = insert(acc, mem[I], index<I> {});
557+
}
558+
return load_masked<Values...>::template apply<I + 1>(mem, acc);
560559
}
561560
};
562561
}
563562

564563
template <class A, class T, bool Value, bool... Values, class Mode>
565-
XSIMD_INLINE batch<T, A> load_masked(T const* mem, batch_bool_constant<T, A, Value, Values...> /* mask */, Mode, requires_arch<neon>) noexcept
564+
XSIMD_INLINE batch<T, A> load_masked(T const* mem, batch_bool_constant<T, A, Value, Values...> /* mask */, convert<T>, Mode, requires_arch<neon>) noexcept
566565
{
567-
// Call insert whenever Values... are true
568-
return detail::load_masked<Values...>::template apply<0>(mem, broadcast(T(0), A {}), std::integral_constant<bool, Value> {});
566+
return detail::load_masked<Value, Values...>::template apply<0>(mem, batch<T, A>(T(0)));
569567
}
570568

571569
/*********

include/xsimd/arch/xsimd_sse2.hpp

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,54 +1100,64 @@ namespace xsimd
11001100
return { load_unaligned(mem, batch_bool<char, A> {}, r).data };
11011101
}
11021102

1103+
namespace detail
1104+
{
1105+
// Plain moves store-forward; vmaskmov never does on Intel and its
1106+
// stores are microcoded on AMD. So wider archs delegate the masks
1107+
// that lower to plain moves here rather than taking their runtime path.
1108+
template <class T, class A, bool... Values>
1109+
constexpr bool lowers_to_plain_moves(batch_bool_constant<T, A, Values...> mask) noexcept
1110+
{
1111+
return (mask.is_prefix() && mask.any() && !mask.all())
1112+
|| mask.suffix() == mask.size / 2;
1113+
}
1114+
}
1115+
11031116
// load_masked
11041117
template <class A, class T, bool... Values, class Mode, class = std::enable_if_t<std::is_integral<T>::value>>
1105-
XSIMD_INLINE batch<T, A> load_masked(T const* mem, batch_bool_constant<T, A, Values...> mask, Mode, requires_arch<sse2>) noexcept
1118+
XSIMD_INLINE batch<T, A> load_masked(T const* mem, batch_bool_constant<T, A, Values...> mask, convert<T>, Mode, requires_arch<sse2>) noexcept
11061119
{
1107-
XSIMD_IF_CONSTEXPR(mask.mask() == 0x1)
1120+
XSIMD_IF_CONSTEXPR(sizeof(T) == 2 && mask.prefix() == 1)
11081121
{
1109-
XSIMD_IF_CONSTEXPR(sizeof(T) == 2)
1110-
{
1111-
return mm_loadu_si16(mem);
1112-
}
1113-
XSIMD_IF_CONSTEXPR(sizeof(T) == 4)
1114-
{
1115-
return mm_loadu_si32(mem);
1116-
}
1117-
XSIMD_IF_CONSTEXPR(sizeof(T) == 8)
1118-
{
1119-
return mm_loadu_si64(mem);
1120-
}
1122+
return _mm_loadu_si16(mem);
1123+
}
1124+
else XSIMD_IF_CONSTEXPR(sizeof(T) == 4 && mask.prefix() == 1)
1125+
{
1126+
return _mm_loadu_si32(mem);
1127+
}
1128+
else XSIMD_IF_CONSTEXPR(sizeof(T) == 8 && mask.prefix() == 1)
1129+
{
1130+
return _mm_loadu_si64(mem);
11211131
}
1122-
else XSIMD_IF_CONSTEXPR(sizeof(T) == 2 && mask.mask() == 0x3)
1132+
else XSIMD_IF_CONSTEXPR(sizeof(T) == 2 && mask.prefix() == 2)
11231133
{
1124-
return mm_loadu_si32(mem);
1134+
return _mm_loadu_si32(mem);
11251135
}
1126-
else XSIMD_IF_CONSTEXPR(sizeof(T) == 4 && mask.mask() == 0x3)
1136+
else XSIMD_IF_CONSTEXPR(sizeof(T) == 4 && mask.prefix() == 2)
11271137
{
1128-
return mm_loadu_si64(mem);
1138+
return _mm_loadu_si64(mem);
11291139
}
11301140
else
11311141
{
11321142
return load_masked<A>(mem, mask, convert<T> {}, Mode {}, common {});
11331143
}
11341144
}
11351145
template <class A, bool... Values, class Mode>
1136-
XSIMD_INLINE batch<float, A> load_masked(float const* mem, batch_bool_constant<float, A, Values...> mask, Mode, requires_arch<sse2>) noexcept
1146+
XSIMD_INLINE batch<float, A> load_masked(float const* mem, batch_bool_constant<float, A, Values...> mask, convert<float>, Mode, requires_arch<sse2>) noexcept
11371147
{
1138-
XSIMD_IF_CONSTEXPR(mask.mask() == 0x1)
1148+
XSIMD_IF_CONSTEXPR(mask.prefix() == 1)
11391149
{
11401150
return _mm_load_ss(mem);
11411151
}
1142-
else XSIMD_IF_CONSTEXPR(mask.countr_one() == 2)
1152+
else XSIMD_IF_CONSTEXPR(mask.prefix() == 2)
11431153
{
11441154
return _mm_loadl_pi(_mm_setzero_ps(), reinterpret_cast<__m64 const*>(mem));
11451155
}
1146-
else XSIMD_IF_CONSTEXPR(mask.countl_one() == 2)
1156+
else XSIMD_IF_CONSTEXPR(mask.suffix() == 2)
11471157
{
11481158
return _mm_loadh_pi(_mm_setzero_ps(), reinterpret_cast<__m64 const*>(mem + 2));
11491159
}
1150-
else XSIMD_IF_CONSTEXPR(mask.countr_one() == 3)
1160+
else XSIMD_IF_CONSTEXPR(mask.prefix() == 3)
11511161
{
11521162
__m128 const lo2 = _mm_castsi128_ps(_mm_loadl_epi64(reinterpret_cast<__m128i const*>(mem)));
11531163
return _mm_shuffle_ps(lo2, _mm_load_ss(mem + 2), _MM_SHUFFLE(3, 0, 1, 0));
@@ -1158,13 +1168,13 @@ namespace xsimd
11581168
}
11591169
}
11601170
template <class A, bool... Values, class Mode>
1161-
XSIMD_INLINE batch<double, A> load_masked(double const* mem, batch_bool_constant<double, A, Values...> mask, Mode, requires_arch<sse2>) noexcept
1171+
XSIMD_INLINE batch<double, A> load_masked(double const* mem, batch_bool_constant<double, A, Values...> mask, convert<double>, Mode, requires_arch<sse2>) noexcept
11621172
{
1163-
XSIMD_IF_CONSTEXPR(mask.countr_one() == 1)
1173+
XSIMD_IF_CONSTEXPR(mask.prefix() == 1)
11641174
{
11651175
return _mm_load_sd(mem);
11661176
}
1167-
else XSIMD_IF_CONSTEXPR(mask.countl_one() == 1)
1177+
else XSIMD_IF_CONSTEXPR(mask.suffix() == 1)
11681178
{
11691179
return _mm_loadh_pd(_mm_setzero_pd(), mem + 1);
11701180
}
@@ -1178,19 +1188,19 @@ namespace xsimd
11781188
template <class A, bool... Values, class Mode>
11791189
XSIMD_INLINE void store_masked(float* mem, batch<float, A> const& src, batch_bool_constant<float, A, Values...> mask, Mode, requires_arch<sse2>) noexcept
11801190
{
1181-
XSIMD_IF_CONSTEXPR(mask.mask() == 0x1)
1191+
XSIMD_IF_CONSTEXPR(mask.prefix() == 1)
11821192
{
11831193
_mm_store_ss(mem, src);
11841194
}
1185-
else XSIMD_IF_CONSTEXPR(mask.countr_one() == 2)
1195+
else XSIMD_IF_CONSTEXPR(mask.prefix() == 2)
11861196
{
11871197
_mm_storel_pi(reinterpret_cast<__m64*>(mem), src);
11881198
}
1189-
else XSIMD_IF_CONSTEXPR(mask.countl_one() == 2)
1199+
else XSIMD_IF_CONSTEXPR(mask.suffix() == 2)
11901200
{
11911201
_mm_storeh_pi(reinterpret_cast<__m64*>(mem + 2), src);
11921202
}
1193-
else XSIMD_IF_CONSTEXPR(mask.countr_one() == 3)
1203+
else XSIMD_IF_CONSTEXPR(mask.prefix() == 3)
11941204
{
11951205
_mm_storel_pi(reinterpret_cast<__m64*>(mem), src);
11961206
_mm_store_ss(mem + 2, _mm_movehl_ps(src, src));
@@ -1204,11 +1214,11 @@ namespace xsimd
12041214
template <class A, bool... Values, class Mode>
12051215
XSIMD_INLINE void store_masked(double* mem, batch<double, A> const& src, batch_bool_constant<double, A, Values...> mask, Mode, requires_arch<sse2>) noexcept
12061216
{
1207-
XSIMD_IF_CONSTEXPR(mask.countr_one() == 1)
1217+
XSIMD_IF_CONSTEXPR(mask.prefix() == 1)
12081218
{
12091219
_mm_store_sd(mem, src);
12101220
}
1211-
else XSIMD_IF_CONSTEXPR(mask.countl_one() == 1)
1221+
else XSIMD_IF_CONSTEXPR(mask.suffix() == 1)
12121222
{
12131223
_mm_storeh_pd(mem + 1, src);
12141224
}
@@ -2331,11 +2341,11 @@ namespace xsimd
23312341
aligned_mode,
23322342
requires_arch<sse2>) noexcept
23332343
{
2334-
XSIMD_IF_CONSTEXPR(mask.countr_one() == 2)
2344+
XSIMD_IF_CONSTEXPR(mask.prefix() == 2)
23352345
{
23362346
_mm_storel_pi(reinterpret_cast<__m64*>(mem), src);
23372347
}
2338-
else XSIMD_IF_CONSTEXPR(mask.countl_one() == 2)
2348+
else XSIMD_IF_CONSTEXPR(mask.suffix() == 2)
23392349
{
23402350
_mm_storeh_pi(reinterpret_cast<__m64*>(mem + 2), src);
23412351
}

include/xsimd/types/xsimd_batch_constant.hpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,34 @@ namespace xsimd
9999
return countl_one_impl(truncated_mask(), size);
100100
}
101101

102+
// true when the set lanes form one contiguous run starting at lane 0
103+
// (the empty and full masks are prefixes)
104+
static constexpr bool is_prefix() noexcept
105+
{
106+
return (truncated_mask() & (truncated_mask() + 1u)) == 0u;
107+
}
108+
109+
// true when the set lanes form one contiguous run ending at the last
110+
// lane (the empty and full masks are suffixes)
111+
static constexpr bool is_suffix() noexcept
112+
{
113+
return ((truncated_mask() ^ low_mask(size)) & ((truncated_mask() ^ low_mask(size)) + 1u)) == 0u;
114+
}
115+
116+
// length of the set run when the mask is a pure prefix (first k lanes
117+
// set, rest clear), else size + 1 so `prefix() == k` stays exact
118+
static constexpr std::size_t prefix() noexcept
119+
{
120+
return is_prefix() ? countr_one() : size + 1;
121+
}
122+
123+
// length of the set run when the mask is a pure suffix (last k lanes
124+
// set, rest clear), else size + 1 so `suffix() == k` stays exact
125+
static constexpr std::size_t suffix() noexcept
126+
{
127+
return is_suffix() ? countl_one() : size + 1;
128+
}
129+
102130
private:
103131
static constexpr int mask_helper(int acc) noexcept { return acc; }
104132

test/test_batch_constant.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,76 @@ struct constant_bool_batch_test
389389
constexpr auto inv_x = ~x;
390390
static_assert(std::is_same<decltype(inv_x), decltype(y)>::value, "~x == y");
391391
}
392+
393+
struct first_half
394+
{
395+
static constexpr bool get(size_t index, size_t size) { return index < size / 2; }
396+
};
397+
struct second_half
398+
{
399+
static constexpr bool get(size_t index, size_t size) { return index >= size / 2; }
400+
};
401+
struct ends
402+
{
403+
static constexpr bool get(size_t index, size_t size) { return index == 0 || index + 1 == size; }
404+
};
405+
struct all_but_last
406+
{
407+
static constexpr bool get(size_t index, size_t size) { return index + 1 < size; }
408+
};
409+
struct all_but_first
410+
{
411+
static constexpr bool get(size_t index, size_t) { return index != 0; }
412+
};
413+
struct first_only
414+
{
415+
static constexpr bool get(size_t index, size_t) { return index == 0; }
416+
};
417+
struct last_only
418+
{
419+
static constexpr bool get(size_t index, size_t size) { return index + 1 == size; }
420+
};
421+
422+
void test_shape() const
423+
{
424+
constexpr auto all_true = xsimd::make_batch_bool_constant<value_type, constant<true>, arch_type>();
425+
constexpr auto all_false = xsimd::make_batch_bool_constant<value_type, constant<false>, arch_type>();
426+
constexpr auto lo = xsimd::make_batch_bool_constant<value_type, first_half, arch_type>();
427+
constexpr auto hi = xsimd::make_batch_bool_constant<value_type, second_half, arch_type>();
428+
constexpr auto edges = xsimd::make_batch_bool_constant<value_type, ends, arch_type>();
429+
constexpr auto size = decltype(all_true)::size;
430+
431+
static_assert(all_true.is_prefix() && all_true.is_suffix(), "full mask is prefix and suffix");
432+
static_assert(all_false.is_prefix() && all_false.is_suffix(), "empty mask is prefix and suffix");
433+
static_assert(lo.is_prefix() && !lo.is_suffix(), "first half is a prefix only");
434+
static_assert(hi.is_suffix() && !hi.is_prefix(), "second half is a suffix only");
435+
// {first, last} lanes: contiguous only when that covers the whole batch
436+
static_assert(edges.is_prefix() == (size <= 2), "non-contiguous mask is not a prefix");
437+
static_assert(edges.is_suffix() == (size <= 2), "non-contiguous mask is not a suffix");
438+
439+
// prefix()/suffix() return the set-run length, or size+1 when not that shape
440+
static_assert(lo.prefix() == size / 2 && lo.suffix() == size + 1, "lo is the size/2 prefix, no suffix");
441+
static_assert(hi.suffix() == size / 2 && hi.prefix() == size + 1, "hi is the size/2 suffix, no prefix");
442+
static_assert(all_true.prefix() == size && all_true.suffix() == size, "full mask runs the whole width");
443+
static_assert(all_false.prefix() == 0 && all_false.suffix() == 0, "empty mask has zero-length runs");
444+
static_assert(edges.prefix() == (size <= 2 ? size : size + 1), "non-contiguous mask has no prefix length");
445+
446+
// off-by-one boundary: the length adjacent to the sentinel (size-1) must
447+
// count exactly, and the sentinel must be exactly size+1 (never size, size+2)
448+
constexpr auto pre1 = xsimd::make_batch_bool_constant<value_type, all_but_last, arch_type>();
449+
constexpr auto suf1 = xsimd::make_batch_bool_constant<value_type, all_but_first, arch_type>();
450+
constexpr auto f1 = xsimd::make_batch_bool_constant<value_type, first_only, arch_type>();
451+
constexpr auto l1 = xsimd::make_batch_bool_constant<value_type, last_only, arch_type>();
452+
static_assert(size < 2 || pre1.prefix() == size - 1, "size-1 prefix counts exactly");
453+
static_assert(size < 2 || pre1.suffix() == size + 1, "a size-1 prefix is not a suffix");
454+
static_assert(size < 2 || suf1.suffix() == size - 1, "size-1 suffix counts exactly");
455+
static_assert(size < 2 || suf1.prefix() == size + 1, "a size-1 suffix is not a prefix");
456+
static_assert(size < 2 || (pre1.suffix() != size && pre1.suffix() != size + 2), "sentinel is exactly size+1");
457+
static_assert(f1.prefix() == 1, "single first lane is the 1-prefix");
458+
static_assert(size < 2 || f1.suffix() == size + 1, "single first lane is not a suffix");
459+
static_assert(l1.suffix() == 1, "single last lane is the 1-suffix");
460+
static_assert(size < 2 || l1.prefix() == size + 1, "single last lane is not a prefix");
461+
}
392462
};
393463

394464
TEST_CASE_TEMPLATE("[constant bool batch]", B, BATCH_INT_TYPES)
@@ -410,5 +480,9 @@ TEST_CASE_TEMPLATE("[constant bool batch]", B, BATCH_INT_TYPES)
410480
{
411481
Test.test_ops();
412482
}
483+
SUBCASE("shape")
484+
{
485+
Test.test_shape();
486+
}
413487
}
414488
#endif

0 commit comments

Comments
 (0)