From b9b6d3f4f252ded56b55447b25610becf3792107 Mon Sep 17 00:00:00 2001 From: Marco Barbone Date: Tue, 4 Aug 2026 17:44:04 -0400 Subject: [PATCH 1/2] refactor: replace swizzle template recursion with C++17 constexpr predicates The swizzle mask predicates were written as recursive class/function templates for the C++14 era. The project moved to C++17 in ea590d54, so they can be plain constexpr functions. is_identity, is_only_from_lo and is_only_from_hi become one-line fold expressions. is_dup_lo/is_dup_hi share a single is_dup_from_half loop -- a fold cannot express them because they compare v[i] against v[i+half]. Deletes get_at, identity_impl, dup_lo_impl, dup_hi_impl, only_from_lo_impl, only_from_hi_impl, get_nth_value, cross_impl, the forwarding wrappers, the unused is_cross_lane overload and the now-unused . Public signatures are unchanged, so no call site moves. Net -135/+28 lines. Verified equivalent to the previous implementation by 27230 static_assert(old(...) == new(...)) checks over 5446 masks (exhaustive for N=2 and N=4 including out-of-range indices, sampled plus structured patterns for N=8/N=16; uint32_t, uint16_t, int8_t, int64_t) under both g++ and clang++. The harness was mutation-tested: an off-by-one counter, a flipped comparison and a dropped range check are caught by 15, 370 and 1266 assertions respectively. Generated asm for ~45 swizzle kernels is byte-identical on sse2, avx2 and skylake-avx512. Adds static_asserts for narrow and signed index types (uint16_t, uint8_t, int8_t, int64_t) and for degenerate N=1/N=2 masks, which the previous tests only covered for uint32_t. --- .../arch/common/xsimd_common_swizzle.hpp | 165 +++--------------- test/test_batch_manip.cpp | 16 ++ 2 files changed, 45 insertions(+), 136 deletions(-) diff --git a/include/xsimd/arch/common/xsimd_common_swizzle.hpp b/include/xsimd/arch/common/xsimd_common_swizzle.hpp index 3db5b6412..e3538edb2 100644 --- a/include/xsimd/arch/common/xsimd_common_swizzle.hpp +++ b/include/xsimd/arch/common/xsimd_common_swizzle.hpp @@ -15,7 +15,6 @@ #include "../../config/xsimd_macros.hpp" #include -#include #include namespace xsimd @@ -27,146 +26,46 @@ namespace xsimd { namespace detail { - // ──────────────────────────────────────────────────────────────────────── - // get_at → the I-th element of the pack - template - struct get_at - { - static constexpr T value = get_at::value; - }; - template - struct get_at - { - static constexpr T value = V0; - }; - - // ──────────────────────────────────────────────────────────────────────── - // identity_impl - template - XSIMD_INLINE constexpr bool identity_impl() noexcept { return true; } - template - XSIMD_INLINE constexpr bool identity_impl() noexcept + // v[i] == i for every i + template + XSIMD_INLINE constexpr bool is_identity() noexcept { - return V0 == static_cast(I) - && identity_impl(); + std::size_t i = 0; + return ((Vs == static_cast(i++)) && ...); } - // ──────────────────────────────────────────────────────────────────────── - // dup_lo_impl - template = 0> - XSIMD_INLINE constexpr bool dup_lo_impl() noexcept { return true; } - - template = 0> - XSIMD_INLINE constexpr bool dup_lo_impl() noexcept + // every index points into the low / high half + template + XSIMD_INLINE constexpr bool is_only_from_lo() noexcept { - return get_at::value < static_cast(N / 2) - && get_at::value == get_at::value - && dup_lo_impl(); + return ((Vs < static_cast(sizeof...(Vs) / 2)) && ...); } - // ──────────────────────────────────────────────────────────────────────── - // dup_hi_impl - template = 0> - XSIMD_INLINE constexpr bool dup_hi_impl() noexcept { return true; } - - template = 0> - XSIMD_INLINE constexpr bool dup_hi_impl() noexcept + template + XSIMD_INLINE constexpr bool is_only_from_hi() noexcept { - return get_at::value >= static_cast(N / 2) - && get_at::value < static_cast(N) - && get_at::value == get_at::value - && dup_hi_impl(); + return ((Vs >= static_cast(sizeof...(Vs) / 2)) && ...); } - // ──────────────────────────────────────────────────────────────────────── - // only_from_lo - template - struct only_from_lo_impl; - - template - struct only_from_lo_impl - { - static constexpr bool value = (Last < (Size / 2)); - }; - - template - struct only_from_lo_impl - { - static constexpr bool value = (First < (Size / 2)) && only_from_lo_impl::value; - }; - - template - constexpr bool is_only_from_lo() - { - return only_from_lo_impl::value; - }; - - // ──────────────────────────────────────────────────────────────────────── - // only_from_hi - template - struct only_from_hi_impl; - - template - struct only_from_hi_impl - { - static constexpr bool value = (Last >= (Size / 2)); - }; - - template - struct only_from_hi_impl - { - static constexpr bool value = (First >= (Size / 2)) && only_from_hi_impl::value; - }; - - template - constexpr bool is_only_from_hi() - { - return only_from_hi_impl::value; - }; - - // ──────────────────────────────────────────────────────────────────────── - // 1) helper to get the I-th value from the Vs pack - template - struct get_nth_value - { - static constexpr uint32_t value = get_nth_value::value; - }; - template - struct get_nth_value<0, Head, Tail...> - { - static constexpr uint32_t value = Head; - }; - - // ──────────────────────────────────────────────────────────────────────── - // 2) recursive cross‐lane test: true if any output‐lane i pulls from the opposite half - template - struct cross_impl - { - // does element I cross? (i.e. i=H) or (i>=H but V::value; - static constexpr bool curr = (I < H ? (Vi >= H) : (Vi < H)); - static constexpr bool next = cross_impl::value; - static constexpr bool value = curr || next; - }; - template - struct cross_impl - { - static constexpr bool value = false; - }; - template - XSIMD_INLINE constexpr bool is_cross_lane() noexcept - { - static_assert(sizeof...(Vs) >= 1, "Need at least one lane"); - return cross_impl<0, sizeof...(Vs), sizeof...(Vs) / 2, Vs...>::value; + // both halves read the same indices, all taken from the Hi ? high : low half + template + XSIMD_INLINE constexpr bool is_dup_from_half() noexcept + { + constexpr std::size_t half = sizeof...(Vs) / 2; + constexpr T lo = Hi ? static_cast(half) : T(0); + constexpr T hi = Hi ? static_cast(sizeof...(Vs)) : static_cast(half); + constexpr T v[] = { Vs... }; + for (std::size_t i = 0; i < half; ++i) + if (v[i] < lo || v[i] >= hi || v[i + half] != v[i]) + return false; + return true; } + template + XSIMD_INLINE constexpr bool is_dup_lo() noexcept { return is_dup_from_half(); } + template + XSIMD_INLINE constexpr bool is_dup_hi() noexcept { return is_dup_from_half(); } + /** * @brief Internal: Check if a swizzle pattern crosses lane boundaries * @@ -202,12 +101,6 @@ namespace xsimd return false; } - template - XSIMD_INLINE constexpr bool is_identity() noexcept { return detail::identity_impl<0, T, Vs...>(); } - template - XSIMD_INLINE constexpr bool is_dup_lo() noexcept { return detail::dup_lo_impl<0, sizeof...(Vs), T, Vs...>(); } - template - XSIMD_INLINE constexpr bool is_dup_hi() noexcept { return detail::dup_hi_impl<0, sizeof...(Vs), T, Vs...>(); } template XSIMD_INLINE constexpr bool is_identity(batch_constant) noexcept { return is_identity(); } template diff --git a/test/test_batch_manip.cpp b/test/test_batch_manip.cpp index 8fdc3cfd2..cb3678d51 100644 --- a/test/test_batch_manip.cpp +++ b/test/test_batch_manip.cpp @@ -51,6 +51,22 @@ namespace xsimd // 4-lane dup-hi (repeat 2..3 twice) static_assert(is_dup_hi(), "4-lane dup_hi failed"); static_assert(!is_dup_lo(), "4-lane dup_lo on dup_hi"); + // ──────────────────────────────────────────────────────────────────────── + // narrow and signed index types, as used by the sse2 / avx / avx512bw kernels + static_assert(is_identity(), "uint16_t identity failed"); + static_assert(is_dup_lo(), "uint16_t dup_lo failed"); + static_assert(is_dup_hi(), "uint16_t dup_hi failed"); + static_assert(is_identity(), "uint8_t identity failed"); + static_assert(is_dup_hi(), "int8_t dup_hi failed"); + static_assert(!is_dup_hi(), "int8_t dup_hi on non-dup"); + static_assert(!is_dup_hi(), "int8_t dup_hi with out-of-range index"); + static_assert(is_only_from_lo(), "int64_t only_from_lo failed"); + static_assert(is_only_from_hi(), "int64_t only_from_hi failed"); + // degenerate pack sizes + static_assert(is_identity(), "1-lane identity failed"); + static_assert(!is_identity(), "1-lane identity on non-zero"); + static_assert(is_dup_lo(), "2-lane dup_lo failed"); + static_assert(is_dup_hi(), "2-lane dup_hi failed"); static_assert(is_cross_lane(), "dup-lo only → crossing"); static_assert(is_cross_lane(), "dup-hi only → crossing"); From d0e479fedbd8ad21ed220145909f06a6c8e3c18c Mon Sep 17 00:00:00 2001 From: Marco Barbone Date: Wed, 5 Aug 2026 10:56:43 -0400 Subject: [PATCH 2/2] refactor: split is_dup_lo/is_dup_hi into named predicates Address review: the shared is_dup_from_half helper hid the intent behind 'v[i] < lo || v[i] >= hi || v[i + half] != v[i]'. Each direction is now a conjunction of named properties (in-range, only-from-half, equal-halves), differing by exactly one term. Adds the negative-index dup_lo case previously covered by the explicit lower bound. --- .../arch/common/xsimd_common_swizzle.hpp | 28 +++++++++++++------ test/test_batch_manip.cpp | 1 + 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/include/xsimd/arch/common/xsimd_common_swizzle.hpp b/include/xsimd/arch/common/xsimd_common_swizzle.hpp index e3538edb2..59c8afdb0 100644 --- a/include/xsimd/arch/common/xsimd_common_swizzle.hpp +++ b/include/xsimd/arch/common/xsimd_common_swizzle.hpp @@ -47,24 +47,36 @@ namespace xsimd return ((Vs >= static_cast(sizeof...(Vs) / 2)) && ...); } - // both halves read the same indices, all taken from the Hi ? high : low half - template - XSIMD_INLINE constexpr bool is_dup_from_half() noexcept + // 0 <= v[i] < N for every i (negative values wrap to a huge size_t) + template + XSIMD_INLINE constexpr bool is_in_range() noexcept + { + return ((static_cast(Vs) < sizeof...(Vs)) && ...); + } + + // v[i] == v[i + N / 2] for every i in the low half + template + XSIMD_INLINE constexpr bool has_equal_halves() noexcept { constexpr std::size_t half = sizeof...(Vs) / 2; - constexpr T lo = Hi ? static_cast(half) : T(0); - constexpr T hi = Hi ? static_cast(sizeof...(Vs)) : static_cast(half); constexpr T v[] = { Vs... }; for (std::size_t i = 0; i < half; ++i) - if (v[i] < lo || v[i] >= hi || v[i + half] != v[i]) + if (v[i] != v[i + half]) return false; return true; } + // both halves read the same indices, all taken from the low / high half template - XSIMD_INLINE constexpr bool is_dup_lo() noexcept { return is_dup_from_half(); } + XSIMD_INLINE constexpr bool is_dup_lo() noexcept + { + return is_in_range() && is_only_from_lo() && has_equal_halves(); + } template - XSIMD_INLINE constexpr bool is_dup_hi() noexcept { return is_dup_from_half(); } + XSIMD_INLINE constexpr bool is_dup_hi() noexcept + { + return is_in_range() && is_only_from_hi() && has_equal_halves(); + } /** * @brief Internal: Check if a swizzle pattern crosses lane boundaries diff --git a/test/test_batch_manip.cpp b/test/test_batch_manip.cpp index cb3678d51..1391749e5 100644 --- a/test/test_batch_manip.cpp +++ b/test/test_batch_manip.cpp @@ -60,6 +60,7 @@ namespace xsimd static_assert(is_dup_hi(), "int8_t dup_hi failed"); static_assert(!is_dup_hi(), "int8_t dup_hi on non-dup"); static_assert(!is_dup_hi(), "int8_t dup_hi with out-of-range index"); + static_assert(!is_dup_lo(), "int8_t dup_lo with negative index"); static_assert(is_only_from_lo(), "int64_t only_from_lo failed"); static_assert(is_only_from_hi(), "int64_t only_from_hi failed"); // degenerate pack sizes