Skip to content

Commit e79f9d3

Browse files
authored
Fix avx512vl_{128,256} correctness bugs + CI default-arch typo (#1357)
* ci: fix $CXX_FLAGS typo dropping CXXFLAGS in Linux arch matrix The avx_128, avx2_128 and avx512vl_256 matrix branches set CXXFLAGS="$CXX_FLAGS -DXSIMD_DEFAULT_ARCH=..." but $CXX_FLAGS is undefined at that point, so any inherited CXXFLAGS is wiped and the intended append never happens. Use $CXXFLAGS, matching the avx512vl_128 branch. This also means the avx512vl_256 job now actually compiles with its intended default arch (previously it silently did not). * fix(avx512vl): correct swizzle recursion, neq NaN, decr_if/incr_if Three correctness bugs on the avx512vl_128 / avx512vl_256 arches, which use k-mask batch_bool registers rather than vector masks: - 16-bit dynamic swizzle recursed infinitely. The sized-2 bridge bitwise_cast<uint16_t>(self) is a no-op for uint16_t, so it re-selected itself (gcc: 'always_inline ... inline-unit-growth limit reached', breaking the full avx512vl_256 test build). Add a uint16_t base that forwards to avx2, mirroring the existing uint8_t base. - neq(NaN, x) returned false: float/double neq used _CMP_NEQ_OQ (ordered, false on NaN). IEEE != needs _CMP_NEQ_UQ, as avx/avx512f already use. - decr_if/incr_if produced garbage (e.g. decr_if(1, all-true)=0x10000). They inherited avx's 'self +/- batch<T>(mask.data)', which assumes a vector batch_bool whose true lanes are all-ones; here mask.data is a k-mask bitfield, so the broadcast is wrong. Delegate to the select-based common implementation. Covered by existing tests (test_decr_if/test_incr_if/test_neq/ test_neq_nan in test_xsimd_api.cpp, and the uint16 swizzle in test_shuffle/test_batch_manip), which now pass on both VL arches under SDE; previously they could not even build (swizzle) or were never exercised because the VL_256 CI job lacked its default arch (see prior commit).
1 parent 0929f1a commit e79f9d3

3 files changed

Lines changed: 48 additions & 7 deletions

File tree

.github/workflows/linux.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,14 @@ jobs:
8383
fi
8484
if [[ '${{ matrix.sys.flags }}' == 'avx_128' ]]; then
8585
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=sandybridge"
86-
CXXFLAGS="$CXX_FLAGS -DXSIMD_DEFAULT_ARCH=avx_128"
86+
CXXFLAGS="$CXXFLAGS -DXSIMD_DEFAULT_ARCH=avx_128"
8787
fi
8888
if [[ '${{ matrix.sys.flags }}' == 'avx2' ]]; then
8989
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=haswell"
9090
fi
9191
if [[ '${{ matrix.sys.flags }}' == 'avx2_128' ]]; then
9292
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=haswell"
93-
CXXFLAGS="$CXX_FLAGS -DXSIMD_DEFAULT_ARCH=avx2_128"
93+
CXXFLAGS="$CXXFLAGS -DXSIMD_DEFAULT_ARCH=avx2_128"
9494
fi
9595
if [[ '${{ matrix.sys.flags }}' == 'sse3' ]]; then
9696
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=nocona"
@@ -104,7 +104,7 @@ jobs:
104104
fi
105105
if [[ '${{ matrix.sys.flags }}' == 'avx512vl_256' ]]; then
106106
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=skylake-avx512"
107-
CXXFLAGS="$CXX_FLAGS -DXSIMD_DEFAULT_ARCH=avx512vl_256"
107+
CXXFLAGS="$CXXFLAGS -DXSIMD_DEFAULT_ARCH=avx512vl_256"
108108
fi
109109
if [[ '${{ matrix.sys.flags }}' == 'avx512pf' ]]; then
110110
CMAKE_EXTRA_ARGS="$CMAKE_EXTRA_ARGS -DTARGET_ARCH=knl"

include/xsimd/arch/xsimd_avx512vl_128.hpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,12 +507,12 @@ namespace xsimd
507507
template <class A>
508508
XSIMD_INLINE batch_bool<float, A> neq(batch<float, A> const& self, batch<float, A> const& other, requires_arch<avx512vl_128>) noexcept
509509
{
510-
return (typename batch_bool<float, A>::register_type)_mm_cmp_ps_mask(self, other, _CMP_NEQ_OQ);
510+
return (typename batch_bool<float, A>::register_type)_mm_cmp_ps_mask(self, other, _CMP_NEQ_UQ);
511511
}
512512
template <class A>
513513
XSIMD_INLINE batch_bool<double, A> neq(batch<double, A> const& self, batch<double, A> const& other, requires_arch<avx512vl_128>) noexcept
514514
{
515-
return (typename batch_bool<double, A>::register_type)_mm_cmp_pd_mask(self, other, _CMP_NEQ_OQ);
515+
return (typename batch_bool<double, A>::register_type)_mm_cmp_pd_mask(self, other, _CMP_NEQ_UQ);
516516
}
517517

518518
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
@@ -637,6 +637,22 @@ namespace xsimd
637637
return select(batch_bool<T, A> { Values... }, true_br, false_br, avx512vl_128 {});
638638
}
639639

640+
// decr_if / incr_if — the inherited avx kernels compute
641+
// `self ± batch<T>(mask.data)`, which assumes a vector batch_bool whose
642+
// true lanes are all-ones. Here batch_bool::data is a k-mask bitfield,
643+
// so that broadcast yields garbage. Delegate to the select-based common
644+
// implementation instead.
645+
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
646+
XSIMD_INLINE batch<T, A> decr_if(batch<T, A> const& self, batch_bool<T, A> const& mask, requires_arch<avx512vl_128>) noexcept
647+
{
648+
return decr_if(self, mask, common {});
649+
}
650+
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
651+
XSIMD_INLINE batch<T, A> incr_if(batch<T, A> const& self, batch_bool<T, A> const& mask, requires_arch<avx512vl_128>) noexcept
652+
{
653+
return incr_if(self, mask, common {});
654+
}
655+
640656
// reciprocal
641657
template <class A>
642658
XSIMD_INLINE batch<float, A>

include/xsimd/arch/xsimd_avx512vl_256.hpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,15 @@ namespace xsimd
374374
{
375375
return swizzle(batch<uint8_t, avx2> { self.data }, batch<uint8_t, avx2> { mask.data }, avx2 {}).data;
376376
}
377+
// No EVEX byte/word permute without AVX512_VBMI, so 16-bit dynamic
378+
// swizzle forwards to avx2 — mirrors the uint8_t base above. Without
379+
// this base the sized-2 bridge below re-selects itself for uint16_t
380+
// and recurses forever (always_inline build failure).
381+
template <class A>
382+
XSIMD_INLINE batch<uint16_t, A> swizzle(batch<uint16_t, A> const& self, batch<uint16_t, A> mask, requires_arch<avx512vl_256>) noexcept
383+
{
384+
return swizzle(batch<uint16_t, avx2> { self.data }, batch<uint16_t, avx2> { mask.data }, avx2 {}).data;
385+
}
377386
template <class A, typename T, detail::enable_sized_t<T, 1> = 0>
378387
XSIMD_INLINE batch<T, A> swizzle(batch<T, A> const& self, batch<uint8_t, A> const& mask, requires_arch<avx512vl_256> req) noexcept
379388
{
@@ -589,12 +598,12 @@ namespace xsimd
589598
template <class A>
590599
XSIMD_INLINE batch_bool<float, A> neq(batch<float, A> const& self, batch<float, A> const& other, requires_arch<avx512vl_256>) noexcept
591600
{
592-
return (typename batch_bool<float, A>::register_type)_mm256_cmp_ps_mask(self, other, _CMP_NEQ_OQ);
601+
return (typename batch_bool<float, A>::register_type)_mm256_cmp_ps_mask(self, other, _CMP_NEQ_UQ);
593602
}
594603
template <class A>
595604
XSIMD_INLINE batch_bool<double, A> neq(batch<double, A> const& self, batch<double, A> const& other, requires_arch<avx512vl_256>) noexcept
596605
{
597-
return (typename batch_bool<double, A>::register_type)_mm256_cmp_pd_mask(self, other, _CMP_NEQ_OQ);
606+
return (typename batch_bool<double, A>::register_type)_mm256_cmp_pd_mask(self, other, _CMP_NEQ_UQ);
598607
}
599608

600609
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
@@ -719,6 +728,22 @@ namespace xsimd
719728
return select(batch_bool<T, A> { Values... }, true_br, false_br, avx512vl_256 {});
720729
}
721730

731+
// decr_if / incr_if — the inherited avx kernels compute
732+
// `self ± batch<T>(mask.data)`, which assumes a vector batch_bool whose
733+
// true lanes are all-ones. Here batch_bool::data is a k-mask bitfield,
734+
// so that broadcast yields garbage. Delegate to the select-based common
735+
// implementation instead.
736+
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
737+
XSIMD_INLINE batch<T, A> decr_if(batch<T, A> const& self, batch_bool<T, A> const& mask, requires_arch<avx512vl_256>) noexcept
738+
{
739+
return decr_if(self, mask, common {});
740+
}
741+
template <class A, class T, class = std::enable_if_t<std::is_integral<T>::value>>
742+
XSIMD_INLINE batch<T, A> incr_if(batch<T, A> const& self, batch_bool<T, A> const& mask, requires_arch<avx512vl_256>) noexcept
743+
{
744+
return incr_if(self, mask, common {});
745+
}
746+
722747
// reciprocal
723748
template <class A>
724749
XSIMD_INLINE batch<float, A>

0 commit comments

Comments
 (0)