diff mbox series

libstdc++: Fix simd<char> conversion for -fno-signed-char for Clang

Message ID 13099020.O9o76ZdvQC@excalibur
State New
Headers show
Series libstdc++: Fix simd<char> conversion for -fno-signed-char for Clang | expand

Commit Message

Matthias Kretz June 3, 2024, 3:31 p.m. UTC
Tested on x86_64-linux-gnu (also -m32 and -mx32), aarch64-linux-gnu, and arm-
linux-gnueabi(hf).

OK for trunk and backports?

----------------------- 8< -----------------------

The special case for Clang in the trait producing a signed integer type
lead to the trait returning 'char' where it should have been 'signed
char'. This workaround was introduced because on Clang the return type
of vector compares was not convertible to '_SimdWrapper<
__int_for_sizeof_t<...' unless '__int_for_sizeof_t<char>' was an alias
for 'char'. In order to not rewrite the complete mask type code (there
is code scattered around the implementation assuming signed integers),
this needs to be 'signed char'; so the special case for Clang needs to
be removed.
The conversion issue is now solved in _SimdWrapper, which now
additionally allows conversion from vector types with compatible
integral type.

Signed-off-by: Matthias Kretz <m.kretz@gsi.de>

libstdc++-v3/ChangeLog:

	PR libstdc++/115308
	* include/experimental/bits/simd.h (__int_for_sizeof): Remove
	special cases for __clang__.
	(_SimdWrapper): Change constructor overload set to allow
	conversion from vector types with integral conversions via bit
	reinterpretation.
---
 libstdc++-v3/include/experimental/bits/simd.h | 45 +++++++++++--------
 1 file changed, 27 insertions(+), 18 deletions(-)


--
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 stdₓ::simd
──────────────────────────────────────────────────────────────────────────

Comments

Jonathan Wakely June 3, 2024, 8:04 p.m. UTC | #1
On Mon, 3 Jun 2024 at 16:31, Matthias Kretz <m.kretz@gsi.de> wrote:
>
> Tested on x86_64-linux-gnu (also -m32 and -mx32), aarch64-linux-gnu, and arm-
> linux-gnueabi(hf).
>
> OK for trunk and backports?

OK for all.


>
> ----------------------- 8< -----------------------
>
> The special case for Clang in the trait producing a signed integer type
> lead to the trait returning 'char' where it should have been 'signed
> char'. This workaround was introduced because on Clang the return type
> of vector compares was not convertible to '_SimdWrapper<
> __int_for_sizeof_t<...' unless '__int_for_sizeof_t<char>' was an alias
> for 'char'. In order to not rewrite the complete mask type code (there
> is code scattered around the implementation assuming signed integers),
> this needs to be 'signed char'; so the special case for Clang needs to
> be removed.
> The conversion issue is now solved in _SimdWrapper, which now
> additionally allows conversion from vector types with compatible
> integral type.
>
> Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/115308
>         * include/experimental/bits/simd.h (__int_for_sizeof): Remove
>         special cases for __clang__.
>         (_SimdWrapper): Change constructor overload set to allow
>         conversion from vector types with integral conversions via bit
>         reinterpretation.
> ---
>  libstdc++-v3/include/experimental/bits/simd.h | 45 +++++++++++--------
>  1 file changed, 27 insertions(+), 18 deletions(-)
>
>
> --
> ──────────────────────────────────────────────────────────────────────────
>  Dr. Matthias Kretz                           https://mattkretz.github.io
>  GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
>  stdₓ::simd
> ──────────────────────────────────────────────────────────────────────────
diff mbox series

Patch

diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h
index 7c524625719..cb1f13d8ba6 100644
--- a/libstdc++-v3/include/experimental/bits/simd.h
+++ b/libstdc++-v3/include/experimental/bits/simd.h
@@ -606,19 +606,12 @@  __int_for_sizeof()
     static_assert(_Bytes > 0);
     if constexpr (_Bytes == sizeof(int))
       return int();
-  #ifdef __clang__
-    else if constexpr (_Bytes == sizeof(char))
-      return char();
-  #else
     else if constexpr (_Bytes == sizeof(_SChar))
       return _SChar();
-  #endif
     else if constexpr (_Bytes == sizeof(short))
       return short();
-  #ifndef __clang__
     else if constexpr (_Bytes == sizeof(long))
       return long();
-  #endif
     else if constexpr (_Bytes == sizeof(_LLong))
       return _LLong();
   #ifdef __SIZEOF_INT128__
@@ -2747,6 +2740,8 @@  struct _SimdWrapperBase<true, _BuiltinType>
 
 // }}}
 // _SimdWrapper{{{
+struct _DisabledSimdWrapper;
+
 template <typename _Tp, size_t _Width>
   struct _SimdWrapper<
     _Tp, _Width,
@@ -2756,16 +2751,17 @@  struct _SimdWrapper
 			      == sizeof(__vector_type_t<_Tp, _Width>),
 		       __vector_type_t<_Tp, _Width>>
   {
-    using _Base
-      = _SimdWrapperBase<__has_iec559_behavior<__signaling_NaN, _Tp>::value
-			   && sizeof(_Tp) * _Width
-				== sizeof(__vector_type_t<_Tp, _Width>),
-			 __vector_type_t<_Tp, _Width>>;
+    static constexpr bool _S_need_default_init
+      = __has_iec559_behavior<__signaling_NaN, _Tp>::value
+	  and sizeof(_Tp) * _Width == sizeof(__vector_type_t<_Tp, _Width>);
+
+    using _BuiltinType = __vector_type_t<_Tp, _Width>;
+
+    using _Base = _SimdWrapperBase<_S_need_default_init, _BuiltinType>;
 
     static_assert(__is_vectorizable_v<_Tp>);
     static_assert(_Width >= 2); // 1 doesn't make sense, use _Tp directly then
 
-    using _BuiltinType = __vector_type_t<_Tp, _Width>;
     using value_type = _Tp;
 
     static inline constexpr size_t _S_full_size
@@ -2801,13 +2797,26 @@  _SimdWrapper(initializer_list<_Tp> __init)
     _GLIBCXX_SIMD_INTRINSIC constexpr _SimdWrapper&
     operator=(_SimdWrapper&&) = default;
 
-    template <typename _V, typename = enable_if_t<disjunction_v<
-			     is_same<_V, __vector_type_t<_Tp, _Width>>,
-			     is_same<_V, __intrinsic_type_t<_Tp, _Width>>>>>
+    // Convert from exactly matching __vector_type_t
+    using _SimdWrapperBase<_S_need_default_init, _BuiltinType>::_SimdWrapperBase;
+
+    // Convert from __intrinsic_type_t if __intrinsic_type_t and __vector_type_t differ, otherwise
+    // this ctor should not exist. Making the argument type unusable is our next best solution.
+    _GLIBCXX_SIMD_INTRINSIC constexpr
+    _SimdWrapper(conditional_t<is_same_v<_BuiltinType, __intrinsic_type_t<_Tp, _Width>>,
+			       _DisabledSimdWrapper, __intrinsic_type_t<_Tp, _Width>> __x)
+    : _Base(__vector_bitcast<_Tp, _Width>(__x)) {}
+
+    // Convert from different __vector_type_t, but only if bit reinterpretation is a correct
+    // conversion of the value_type
+    template <typename _V, typename _TVT = _VectorTraits<_V>,
+	      typename = enable_if_t<sizeof(typename _TVT::value_type) == sizeof(_Tp)
+				       and sizeof(_V) == sizeof(_BuiltinType)
+				       and is_integral_v<_Tp>
+				       and is_integral_v<typename _TVT::value_type>>>
       _GLIBCXX_SIMD_INTRINSIC constexpr
       _SimdWrapper(_V __x)
-      // __vector_bitcast can convert e.g. __m128 to __vector(2) float
-      : _Base(__vector_bitcast<_Tp, _Width>(__x)) {}
+      : _Base(reinterpret_cast<_BuiltinType>(__x)) {}
 
     template <typename... _As,
 	      typename = enable_if_t<((is_same_v<simd_abi::scalar, _As> && ...)