Message ID | orlevjgyu5.fsf_-_@lxoliva.fsfla.org |
---|---|
State | New |
Headers | show |
Series | [v2] libstdc++: ppc: conditionalize vsx-only simd intrinsics | expand |
On Mon, May 02, 2022 at 11:00:02PM -0300, Alexandre Oliva wrote: > On May 2, 2022, Segher Boessenkool <segher@kernel.crashing.org> wrote: > > Send full patches always please. > > I'll try to remember that. In case I fail, I hope you won't mind too > much reminding me. > > (You'd also asked me not to send patches as followups, but... revised > versions of a patch still belong in the same thread, right?) No. This makes it much harder to keep versions apart, even worse if you use patchwork or similar. The mail with the patch should be the head of a mail thread (or just below the head if that is a cover mail), and everything below that is discussion related to that patch. The mail with the patch should be ready to be committed as-is, so it should not have mangled whitespace or encoding, should have proper commit message, etc. This a) makes it possible to review what will actualle be committed, and b) it makes it possible for someone else to commit it (and that includes patch test systems, often mistakenly called CI or CD, which are completely different things). > libstdc++'s bits/simd.h section for PPC (Altivec) defines various (Spelling: PowerPC, AltiVec. But this isn't about AltiVec really anyway?) > intrinsic vector types that are only available along with VSX: 64-bit > long double, double, (un)signed long long, and 64-bit (un)signed long. > +#if defined __VSX__ || __LONG_WIDTH__ == 32 > _GLIBCXX_SIMD_PPC_INTRIN(signed long); > _GLIBCXX_SIMD_PPC_INTRIN(unsigned long); > +#endif Is __LONG_WIDTH__ the right macro to use here? Nothing else in libstdc++v3 uses it. "__CHAR_BIT__ * __SIZEOF_LONG__" is the usual thing to do. Is __LONG_WIDTH__ always defined anyway? > @@ -2450,10 +2456,11 @@ template <typename _Tp, size_t _Bytes> > static constexpr bool _S_is_ldouble = is_same_v<_Tp, long double>; > // allow _Tp == long double with -mlong-double-64 > static_assert(!(_S_is_ldouble && sizeof(long double) > sizeof(double)), > - "no __intrinsic_type support for long double on PPC"); > + "no __intrinsic_type support for 128-bit floating point on PPC"); You might do s/PPC/PowerPC/ at the same time. Okay for trunk with __LONG_WIDTH__ dealt with. Okay for the branches a week or so after that. Thanks! Segher
On Fri, 6 May 2022 at 17:17, Segher Boessenkool wrote: > > +#if defined __VSX__ || __LONG_WIDTH__ == 32 > > _GLIBCXX_SIMD_PPC_INTRIN(signed long); > > _GLIBCXX_SIMD_PPC_INTRIN(unsigned long); > > +#endif > > Is __LONG_WIDTH__ the right macro to use here? Nothing else in > libstdc++v3 uses it. "__CHAR_BIT__ * __SIZEOF_LONG__" is the usual > thing to do. Is __LONG_WIDTH__ always defined anyway? Presumably it could be simply __SIZEOF_LONG__ == 4 if this is PowerPC-specific code where CHAR_BIT==8 is always true? We don't need to consider hypothetical targets where CHAR_BIT!=8 if we already know the target is some version of PowerPC.
On May 6, 2022, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On Fri, 6 May 2022 at 17:17, Segher Boessenkool wrote: >> > +#if defined __VSX__ || __LONG_WIDTH__ == 32 >> > _GLIBCXX_SIMD_PPC_INTRIN(signed long); >> > _GLIBCXX_SIMD_PPC_INTRIN(unsigned long); >> > +#endif >> >> Is __LONG_WIDTH__ the right macro to use here? Nothing else in >> libstdc++v3 uses it. "__CHAR_BIT__ * __SIZEOF_LONG__" is the usual >> thing to do. Is __LONG_WIDTH__ always defined anyway? > Presumably it could be simply __SIZEOF_LONG__ == 4 if this is > PowerPC-specific code where CHAR_BIT==8 is always true? SGTM. Here's the adjusted patch I'm checking in momentarily, trunk first, then gcc-12 and gcc-11 after a week or so. Thanks, libstdc++: ppc: conditionalize vsx-only simd intrinsics libstdc++'s bits/simd.h section for PowerPC, guarded by __ALTIVEC__, defines various intrinsic vector types that are only available with __VSX__: 64-bit long double, double, (un)signed long long, and 64-bit (un)signed long. experimental/simd/standard_abi_usable{,_2}.cc tests error out reporting the unmet requirements when the target cpu doesn't enable VSX. Make the reported instrinsic types conditional on __VSX__ so that <experimental/simd> can be used on PowerPC variants that do not support VSX. for libstdc++-v3/ChangeLog * include/experimental/bits/simd.h [__ALTIVEC__]: Require VSX for double, long long, and 64-bit long intrinsic types. [__ALTIVEC__] (__intrinsic_type): Mention 128-bit in preexisting long double diagnostic, adjust no-VSX double diagnostic to cover 64-bit long double as well. --- libstdc++-v3/include/experimental/bits/simd.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h index 82e9841195e1d..b0226fa4c5304 100644 --- a/libstdc++-v3/include/experimental/bits/simd.h +++ b/libstdc++-v3/include/experimental/bits/simd.h @@ -2430,17 +2430,23 @@ template <typename _Tp> template <> \ struct __intrinsic_type_impl<_Tp> { using type = __vector _Tp; } _GLIBCXX_SIMD_PPC_INTRIN(float); +#ifdef __VSX__ _GLIBCXX_SIMD_PPC_INTRIN(double); +#endif _GLIBCXX_SIMD_PPC_INTRIN(signed char); _GLIBCXX_SIMD_PPC_INTRIN(unsigned char); _GLIBCXX_SIMD_PPC_INTRIN(signed short); _GLIBCXX_SIMD_PPC_INTRIN(unsigned short); _GLIBCXX_SIMD_PPC_INTRIN(signed int); _GLIBCXX_SIMD_PPC_INTRIN(unsigned int); +#if defined __VSX__ || __SIZEOF_LONG__ == 4 _GLIBCXX_SIMD_PPC_INTRIN(signed long); _GLIBCXX_SIMD_PPC_INTRIN(unsigned long); +#endif +#ifdef __VSX__ _GLIBCXX_SIMD_PPC_INTRIN(signed long long); _GLIBCXX_SIMD_PPC_INTRIN(unsigned long long); +#endif #undef _GLIBCXX_SIMD_PPC_INTRIN template <typename _Tp, size_t _Bytes> @@ -2450,10 +2456,11 @@ template <typename _Tp, size_t _Bytes> static constexpr bool _S_is_ldouble = is_same_v<_Tp, long double>; // allow _Tp == long double with -mlong-double-64 static_assert(!(_S_is_ldouble && sizeof(long double) > sizeof(double)), - "no __intrinsic_type support for long double on PPC"); + "no __intrinsic_type support for 128-bit floating point on PowerPC"); #ifndef __VSX__ - static_assert(!is_same_v<_Tp, double>, - "no __intrinsic_type support for double on PPC w/o VSX"); + static_assert(!(is_same_v<_Tp, double> + || (_S_is_ldouble && sizeof(long double) == sizeof(double))), + "no __intrinsic_type support for 64-bit floating point on PowerPC w/o VSX"); #endif using type = typename __intrinsic_type_impl<
diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h index 82e9841195e1d..349726a16d71c 100644 --- a/libstdc++-v3/include/experimental/bits/simd.h +++ b/libstdc++-v3/include/experimental/bits/simd.h @@ -2430,17 +2430,23 @@ template <typename _Tp> template <> \ struct __intrinsic_type_impl<_Tp> { using type = __vector _Tp; } _GLIBCXX_SIMD_PPC_INTRIN(float); +#ifdef __VSX__ _GLIBCXX_SIMD_PPC_INTRIN(double); +#endif _GLIBCXX_SIMD_PPC_INTRIN(signed char); _GLIBCXX_SIMD_PPC_INTRIN(unsigned char); _GLIBCXX_SIMD_PPC_INTRIN(signed short); _GLIBCXX_SIMD_PPC_INTRIN(unsigned short); _GLIBCXX_SIMD_PPC_INTRIN(signed int); _GLIBCXX_SIMD_PPC_INTRIN(unsigned int); +#if defined __VSX__ || __LONG_WIDTH__ == 32 _GLIBCXX_SIMD_PPC_INTRIN(signed long); _GLIBCXX_SIMD_PPC_INTRIN(unsigned long); +#endif +#ifdef __VSX__ _GLIBCXX_SIMD_PPC_INTRIN(signed long long); _GLIBCXX_SIMD_PPC_INTRIN(unsigned long long); +#endif #undef _GLIBCXX_SIMD_PPC_INTRIN template <typename _Tp, size_t _Bytes> @@ -2450,10 +2456,11 @@ template <typename _Tp, size_t _Bytes> static constexpr bool _S_is_ldouble = is_same_v<_Tp, long double>; // allow _Tp == long double with -mlong-double-64 static_assert(!(_S_is_ldouble && sizeof(long double) > sizeof(double)), - "no __intrinsic_type support for long double on PPC"); + "no __intrinsic_type support for 128-bit floating point on PPC"); #ifndef __VSX__ - static_assert(!is_same_v<_Tp, double>, - "no __intrinsic_type support for double on PPC w/o VSX"); + static_assert(!(is_same_v<_Tp, double> + || (_S_is_ldouble && sizeof(long double) == sizeof(double))), + "no __intrinsic_type support for 64-bit floating point on PPC w/o VSX"); #endif using type = typename __intrinsic_type_impl<