Skip to content

Experiment with if constexpr - #1394

Draft
AntoinePrv wants to merge 2 commits into
xtensor-stack:masterfrom
AntoinePrv:v15-layout
Draft

Experiment with if constexpr#1394
AntoinePrv wants to merge 2 commits into
xtensor-stack:masterfrom
AntoinePrv:v15-layout

Conversation

@AntoinePrv

@AntoinePrv AntoinePrv commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Some ideas experimenting with a fully if constexpr structure by architecture family related to #1267.
The interesting files to look at are the new ones, especially in arithmetic (the v15 is temporary).
We can ignore the CI failure for now.

Some ideas that shaped this:

  • No native SIMD types (_mm...) in helper function signature as they can cause overloading issues, preferring batch. Ideally providing similar helpers across architectures.
  • All kernel functions (well only add here) have a single entry point (maybe two later with batch_constant).
  • It is forward declared in kernel_fwd to make it easier to build shared utilities and break circular headers.
  • No more requires_arch needed
  • I separated intrinsic overload from dispatching.

What is nice:

  • Safe intrinsic overloads with a batch API are error proof: they will fail to compile if the operation is not supported on the batch.
  • Clear dispatch, e.g. in this add implementation on x86, you immediately see that by going from avx to avx2 you get native support on integers, which is something some user might want to take into account when selecting the arch they target.
    template <class T, class A>
    XSIMD_INLINE batch<T, A> add(batch<T, A> lhs, batch<T, A> rhs) noexcept
    {
      constexpr auto recurse_add = [](auto l, auto r) { return kernel::add(l, r); };
      if constexpr (std::is_base_of_v<avx512f, A>) {
          if constexpr (!std::is_base_of_v<avx512bw, A> && std::is_integral_v<T> && sizeof(T) <= 2) {
              return detail::apply_on_halves(recurse_add, lhs, rhs);
          } else {
              return detail::mm512_add(lhs, rhs);
          }
      } else if constexpr (std::is_base_of_v<avx, A>) {
          if constexpr (std::is_integral_v<T> && std::is_base_of_v<avx2, A>) {
              return detail::mm256_add(lhs, rhs);
          } else {
              return detail::apply_on_halves(recurse_add, lhs, rhs);
          }
      } else if constexpr (std::is_base_of_v<sse2, A>) {  // SSE family and avx<N>_128
          return detail::mm_add(lhs, rhs);
      } else {
          detail::unsupported<T, A>();
      }
    }

The part I really like is the split between the intrinsic wrappers (mm_add, mm256_add, etc) and the dispatch.
It feels like a correct abstraction because what you find inside these functions is very predictable.
With this, it starts looking even more closely to the "recipes" that @serge-sans-paille mentioned.
These overloads and some of these dispatch rules are getting close to being generated from data / recipe.

  • Collect the data of intrinsic and their supported architecture,
  • Describe the architecture hierarchy,
  • Provide what (now overloaded) intrinsic to use,
  • Or provide what fallback strategy to use (forward to other arch, call other functions...).

Possibly reuse that data to generate xsimd for Rust, C...

What is not so nice:

  • In this case some condition are actually repeated between kernel::add and kernel::detail::mm512_add

  • Some ADL gotchas with template, e.g. SVE svptrue<T> utility being a template, we *do need a foward declaration to use it in add on Neon even if is in a discarded if constexpr branch.

  • More verbose, and more LOC, though this can be reduced by changing the formater from

    if(...)
    {
      ...
    }
    else
    {
      ...
    }

    To

    if(...) {
      ...
    } else {
      ...
    }

    Where should we go?
    First this is still some large code changes. It would be mistake IMHO to start a migration if we do not have a reasonable hope of finishing it (risking a never-ending migration). This was the first function so it took some more time to set up (still not fully building), but this is not the hardest function.

I think we may first need to focus on merging overloads with the given structure (e.g. replacing all use of std::enable_if on functions to instead use if constexpr). Removing more C++14 constructs. Then, focus on providing forward declaration to provide more code reuse. Finally merge the remaining arch overloads.

In parallel we can start a data-driven approach to generate safe intrinsic wrappers.

@AntoinePrv

Copy link
Copy Markdown
Contributor Author

What are your thoughts @JohanMabille @serge-sans-paille @DiamonDinoia ?

@DiamonDinoia

Copy link
Copy Markdown
Contributor

Hi @AntoinePrv,

Some suggestions for the discussion. I would implement it like this:

namespace detail {
template <class T, class A> inline constexpr bool has_mm_add =
    std::is_base_of_v<sse2, A> && std::is_arithmetic_v<T>;

template <class T, class A> inline constexpr bool has_mm256_add =
    std::is_base_of_v<avx, A> && (std::is_floating_point_v<T> || std::is_base_of_v<avx2, A>);

template <class T, class A> inline constexpr bool has_mm512_add =
    std::is_base_of_v<avx512f, A>
    && (std::is_floating_point_v<T> || sizeof(T) >= 4 || std::is_base_of_v<avx512bw, A>);
}

template <class T, class A>
XSIMD_INLINE batch<T, A> add(batch<T, A> lhs, batch<T, A> rhs) noexcept
{
    constexpr auto recurse_add = [](auto l, auto r) { return kernel::add(l, r); };

    if constexpr (detail::has_mm512_add<T, A>)      return detail::mm512_add(lhs, rhs);
    else if constexpr (detail::has_mm256_add<T, A>) return detail::mm256_add(lhs, rhs);
    else if constexpr (detail::has_mm_add<T, A>)    return detail::mm_add(lhs, rhs);
    else if constexpr (std::is_base_of_v<avx512f, A> || std::is_base_of_v<avx, A>)
        return detail::apply_on_halves(recurse_add, lhs, rhs);
    else detail::unsupported<T, A>();
}

The approach you proposed has a bug. It is easy to make the mistake::

batch<float, avx> takes the else of the inner if constexpr (std::is_integral_v && std::is_base_of_v<avx2, A>) and goes to apply_on_halves -> two _mm_add_ps instead of one _mm256_add_ps

An alternative which I like (I am biased here (https://poet.readthedocs.io/en/latest/):

I vibe coded the following idea: https://github.com/DiamonDinoia/xsimd/tree/proto/if-constexpr-dispatch

I do not have a preference yet for what is best. I want to avoid churning for the sake of churning so I would like us to land on something that meaningfully improves maintainability/debugging.

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.

2 participants