Re: [PATCH v2] libstdc++: add ARM SVE support to std::experimental::simd

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 27 Mar 2024 at 12:13, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Matthias Kretz <m.kretz@gsi.de> writes:
> > On Wednesday, 27 March 2024 11:07:14 CET Richard Sandiford wrote:
> >> I'm still worried about:
> >>
> >>   #if _GLIBCXX_SIMD_HAVE_SVE
> >>   constexpr inline int __sve_vectorized_size_bytes = __ARM_FEATURE_SVE_BITS
> >> / 8; #else
> >>   constexpr inline int __sve_vectorized_size_bytes = 0;
> >>   #endif
> >>
> >> and the direct use __ARM_FEATURE_SVE_BITS elsewhere, for the reasons
> >> discussed here (including possible ODR problems):
> >>
> >>   https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640037.html
> >>   https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643734.html
> >>
> >> Logically the vector length should be a template parameter rather than
> >> an invariant.  Has this been resolved?  If not, it feels like a blocker
> >> to me (sorry).
> >
> > The vector length is always a template parameter to all user-facing API. Some
> > examples
> >
> > 1. on aarch64 the following is independent of SVE flags (and status quo):
> >
> >   simd<float> is an alias for
> >   simd<float, simd_abi::_VecBuiltin<16>
> >
> >   fixed_size_simd<float, 4> is supposed to be ABI-stable anyway (passed via
> >   the stack, alignof == sizeof).
> >
> > 2. with -msve-vector-bits=512:
> >
> >   native_simd<float> is an alias for
> >   simd<float, simd_abi::_SveAbi<64, 64>>
> >
> >   simd<float, simd_abi::deduce_t<float, 4>> is an alias for
> >   simd<float, simd_abi::_SveAbi<16, 64>>
> >
> > 3. with -msve-vector-bits=256:
> >
> >   native_simd<float> is an alias for
> >   simd<float, simd_abi::_SveAbi<32, 32>>
> >
> >   simd<float, simd_abi::deduce_t<float, 4>> is an alias for
> >   simd<float, simd_abi::_SveAbi<16, 32>>
> >
> > Implementation functions are either [[gnu::always_inline]] or tagged with the
> > ABI tag type and the __odr_helper template argument (to ensure not-inlined
> > inline functions have unique names).
>
> Ah, thanks for the explanation.  I think the global native_float alias
> is problematic for reasons that you touched on in your later message.
> I'll reply more about that there.  But in other respects this looks good.
>
> > Does that make __ARM_FEATURE_SVE_BITS usage indirect enough?
>
> In principle, the only use of __ARM_FEATURE_SVE_BITS should be to determine
> the definition of native_simd (with the caveats above).  But current
> GCC restrictions might make that impractical.
>
> > Also for context, please consider that this is std::*experimental*::simd. The
> > underlying ISO document will likely get retracted at some point and the whole
> > API and implementation (hopefully) superseded by C++26. The main purpose of
> > the spec and implementation is to gather experience.
>
> Ah, ok.  If this is a deliberate experiment for evidence-gathering
> purposes, rather than a long-term commitment, then I agree the barrier
> should be lower.

Yes, that's definitely what this code is for. The more feedback and
impl-experience we can get now with the std::experimental::simd
version, the better std::simd will be when that happens.

In practice, we probably won't ever actually remove the
<experimental/simd> header even when the experiment is over (e.g. we
still have <tr1/memory> with std::tr1::shared_ptr!), but we are likely
to consider it unmaintained and deprecated once it's superseded by
std::simd.

> So yeah, I'll withdraw my objection.  I've no problem with this going
> into GCC 14 on the basis above.  Thanks again to you and Srinivas for
> working on this.
>
> Richard
>