_MaskImplNeon::_S_popcount initialises NEON intrinsic-typed
temporaries (int8x8_t, int16x4_t, int32x2_t) directly from generic
__vector values. On 32-bit Arm the intrinsic vector types are
distinct from the generic __vector(N) T types, so the C++ front end
rejects the conversion and the popcount path fails to compile, e.g.:
simd_neon.h:483: error: cannot convert '__vector(8) signed char'
to 'int8x8_t'
This is the same class of error fixed for _S_to_bits by wrapping the
vpadd_* operands in __to_intrin, and complements that fix: __to_intrin
is a same-size reinterpret, so this is a pure type correction with no
change to the computed result. AArch64 is unaffected because there
the two vector type families coincide.
libstdc++-v3/ChangeLog:
PR libstdc++/122981
* include/experimental/bits/simd_neon.h (_MaskImplNeon::
_S_popcount): Convert the pairwise-add temporaries with
__to_intrin.
* testsuite/experimental/simd/pr122981_popcount_neon.cc:
New test.
Signed-off-by: Dominic P <[email protected]>
---
Hi Torbjörn,
We hit the same failures independently on arm-none-eabi and had an
equivalent fix queued, so: your patch matches what we tested for the
_S_to_bits paths, and with it applied pr115454_find_last_set.cc
compiles and passes here too.
There is one more instance of the same class of error that it does not
cover: _MaskImplNeon::_S_popcount (around line 481) initialises NEON
intrinsic-typed temporaries directly from generic __vector values,
int8x8_t __tmp = __lo64(__s8) + __hi64z(__s8);
which the front end rejects on 32-bit Arm for the same reason, so the
popcount path still fails to compile after your patch. Below is a
complementary patch (it applies independently of yours - the hunks are
disjoint) wrapping those three temporaries in __to_intrin, plus a
popcount test in the style of your find_last_set one.
Tested on arm-none-eabi (armv7-a+neon): the new popcount test fails to
compile without the patch and compiles with it. AArch64 is unaffected
by construction - there the intrinsic and generic vector types
coincide, so __to_intrin is the identity there, as with your patch.
Dominic
.../include/experimental/bits/simd_neon.h | 6 ++--
.../simd/pr122981_popcount_neon.cc | 31 +++++++++++++++++++
2 files changed, 34 insertions(+), 3 deletions(-)
create mode 100644
libstdc++-v3/testsuite/experimental/simd/pr122981_popcount_neon.cc
diff --git a/libstdc++-v3/include/experimental/bits/simd_neon.h
b/libstdc++-v3/include/experimental/bits/simd_neon.h
index 8fabee7f3..f8cb1a66a 100644
--- a/libstdc++-v3/include/experimental/bits/simd_neon.h
+++ b/libstdc++-v3/include/experimental/bits/simd_neon.h
@@ -481,20 +481,20 @@ template <typename _Abi, typename>
if constexpr (sizeof(_Tp) == 1)
{
const auto __s8 = __vector_bitcast<_SChar>(__k._M_data);
- int8x8_t __tmp = __lo64(__s8) + __hi64z(__s8);
+ int8x8_t __tmp = __to_intrin(__lo64(__s8) + __hi64z(__s8));
return -vpadd_s8(vpadd_s8(vpadd_s8(__tmp, int8x8_t()), int8x8_t()),
int8x8_t())[0];
}
else if constexpr (sizeof(_Tp) == 2)
{
const auto __s16 = __vector_bitcast<short>(__k._M_data);
- int16x4_t __tmp = __lo64(__s16) + __hi64z(__s16);
+ int16x4_t __tmp = __to_intrin(__lo64(__s16) + __hi64z(__s16));
return -vpadd_s16(vpadd_s16(__tmp, int16x4_t()), int16x4_t())[0];
}
else if constexpr (sizeof(_Tp) == 4)
{
const auto __s32 = __vector_bitcast<int>(__k._M_data);
- int32x2_t __tmp = __lo64(__s32) + __hi64z(__s32);
+ int32x2_t __tmp = __to_intrin(__lo64(__s32) + __hi64z(__s32));
return -vpadd_s32(__tmp, int32x2_t())[0];
}
else if constexpr (sizeof(_Tp) == 8)
diff --git a/libstdc++-v3/testsuite/experimental/simd/pr122981_popcount_neon.cc
b/libstdc++-v3/testsuite/experimental/simd/pr122981_popcount_neon.cc
new file mode 100644
index 000000000..05884e27e
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/simd/pr122981_popcount_neon.cc
@@ -0,0 +1,31 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target { arm*-*-* } } }
+// { dg-require-effective-target arm_neon_ok }
+// { dg-add-options arm_neon }
+// PR libstdc++/122981: _MaskImplNeon::_S_popcount initialised NEON
+// intrinsic-typed temporaries (int8x8_t etc.) from generic __vector
+// values, which the C++ front end rejects on 32-bit Arm where the
+// intrinsic and generic vector types are distinct.
+
+#include <experimental/simd>
+
+namespace stdx = std::experimental;
+
+template <typename T>
+ int
+ test_popcount ()
+ {
+ using M = typename stdx::native_simd<T>::mask_type;
+ M k(true);
+ return stdx::popcount (k);
+ }
+
+int
+main ()
+{
+ int n = 0;
+ n += test_popcount<signed char> ();
+ n += test_popcount<short> ();
+ n += test_popcount<int> ();
+ return n == 0;
+}
--
2.55.0
