libstdc++: Fix <numeric> parallel algos for move-only values [PR117905]

All of reduce, transform_reduce, exclusive_scan, and inclusive_scan,
transform_exclusive_scan, and transform_inclusive_scan have a
precondition that the type of init meets the Cpp17MoveConstructible
requirements. It isn't required to be copy constructible, so when
passing it to the next internal function it needs to be moved, not
copied. We also need to move when creating local variables on the stack,
and when returning as part of a pair.

libstdc++-v3/ChangeLog:

	PR libstdc++/117905
	* include/pstl/glue_numeric_impl.h (reduce, transform_reduce)
	(transform_reduce, inclusive_scan, transform_exclusive_scan)
	(transform_inclusive_scan): Use std::move for __init parameter.
	* include/pstl/numeric_impl.h (__brick_transform_reduce)
	(__pattern_transform_reduce, __brick_transform_scan)
	(__pattern_transform_scan): Likewise.
	* include/std/numeric (inclusive_scan, transform_exclusive_scan):
	Use std::move to create local copy of the first element.
	* testsuite/26_numerics/pstl/numeric_ops/108236.cc: Move test
	using move-only type to ...
	* testsuite/26_numerics/pstl/numeric_ops/move_only.cc: New test.

diff --git a/libstdc++-v3/include/pstl/glue_numeric_impl.h b/libstdc++-v3/include/pstl/glue_numeric_impl.h
index 10d4912deedea1f66919e20fe6d32c5afe367500..fe2d0fd47e24facbb58ccde67f68ab6c6a5339c4 100644 (file)
--- a/libstdc++-v3/include/pstl/glue_numeric_impl.h
+++ b/libstdc++-v3/include/pstl/glue_numeric_impl.h
@@ -25,7 +25,7 @@ __pstl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _Tp>
 reduce(_ExecutionPolicy&& __exec, _ForwardIterator __first, _ForwardIterator __last, _Tp __init,
        _BinaryOperation __binary_op)
 {
-    return transform_reduce(std::forward<_ExecutionPolicy>(__exec), __first, __last, __init, __binary_op,
+    return transform_reduce(std::forward<_ExecutionPolicy>(__exec), __first, __last, std::move(__init), __binary_op,
                             __pstl::__internal::__no_op());
 }
 
@@ -33,7 +33,7 @@ template <class _ExecutionPolicy, class _ForwardIterator, class _Tp>
 __pstl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _Tp>
 reduce(_ExecutionPolicy&& __exec, _ForwardIterator __first, _ForwardIterator __last, _Tp __init)
 {
-    return transform_reduce(std::forward<_ExecutionPolicy>(__exec), __first, __last, __init, std::plus<_Tp>(),
+    return transform_reduce(std::forward<_ExecutionPolicy>(__exec), __first, __last, std::move(__init), std::plus<_Tp>(),
                             __pstl::__internal::__no_op());
 }
 
@@ -58,7 +58,7 @@ transform_reduce(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _Forward
 
     typedef typename iterator_traits<_ForwardIterator1>::value_type _InputType;
     return __pstl::__internal::__pattern_transform_reduce(__dispatch_tag, std::forward<_ExecutionPolicy>(__exec),
-                                                          __first1, __last1, __first2, __init, std::plus<_InputType>(),
+                                                          __first1, __last1, __first2, std::move(__init), std::plus<_InputType>(),
                                                           std::multiplies<_InputType>());
 }
 
@@ -70,7 +70,7 @@ transform_reduce(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _Forward
 {
     auto __dispatch_tag = __pstl::__internal::__select_backend(__exec, __first1, __first2);
     return __pstl::__internal::__pattern_transform_reduce(__dispatch_tag, std::forward<_ExecutionPolicy>(__exec),
-                                                          __first1, __last1, __first2, __init, __binary_op1,
+                                                          __first1, __last1, __first2, std::move(__init), __binary_op1,
                                                           __binary_op2);
 }
 
@@ -81,7 +81,7 @@ transform_reduce(_ExecutionPolicy&& __exec, _ForwardIterator __first, _ForwardIt
 {
     auto __dispatch_tag = __pstl::__internal::__select_backend(__exec, __first);
     return __pstl::__internal::__pattern_transform_reduce(__dispatch_tag, std::forward<_ExecutionPolicy>(__exec),
-                                                          __first, __last, __init, __binary_op, __unary_op);
+                                                          __first, __last, std::move(__init), __binary_op, __unary_op);
 }
 
 // [exclusive.scan]
@@ -139,7 +139,7 @@ inclusive_scan(_ExecutionPolicy&& __exec, _ForwardIterator1 __first, _ForwardIte
                _ForwardIterator2 __result, _BinaryOperation __binary_op, _Tp __init)
 {
     return transform_inclusive_scan(std::forward<_ExecutionPolicy>(__exec), __first, __last, __result, __binary_op,
-                                    __pstl::__internal::__no_op(), __init);
+                                    __pstl::__internal::__no_op(), std::move(__init));
 }
 
 // [transform.exclusive.scan]
@@ -154,7 +154,7 @@ transform_exclusive_scan(_ExecutionPolicy&& __exec, _ForwardIterator1 __first, _
     auto __dispatch_tag = __pstl::__internal::__select_backend(__exec, __first, __result);
 
     return __pstl::__internal::__pattern_transform_scan(__dispatch_tag, std::forward<_ExecutionPolicy>(__exec), __first,
-                                                        __last, __result, __unary_op, __init, __binary_op,
+                                                        __last, __result, __unary_op, std::move(__init), __binary_op,
                                                         /*inclusive=*/std::false_type());
 }
 
@@ -170,7 +170,7 @@ transform_inclusive_scan(_ExecutionPolicy&& __exec, _ForwardIterator1 __first, _
     auto __dispatch_tag = __pstl::__internal::__select_backend(__exec, __first, __result);
 
     return __pstl::__internal::__pattern_transform_scan(__dispatch_tag, std::forward<_ExecutionPolicy>(__exec), __first,
-                                                        __last, __result, __unary_op, __init, __binary_op,
+                                                        __last, __result, __unary_op, std::move(__init), __binary_op,
                                                         /*inclusive=*/std::true_type());
 }
 

diff --git a/libstdc++-v3/include/pstl/numeric_impl.h b/libstdc++-v3/include/pstl/numeric_impl.h
index e1ebec1603985905576895e9ebfa3beea1c4cb2d..b285a667653ad06239782874c4b94c0c6e3f624a 100644 (file)
--- a/libstdc++-v3/include/pstl/numeric_impl.h
+++ b/libstdc++-v3/include/pstl/numeric_impl.h
@@ -35,7 +35,7 @@ __brick_transform_reduce(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
                          _BinaryOperation1 __binary_op1, _BinaryOperation2 __binary_op2,
                          /*is_vector=*/std::false_type) noexcept
 {
-    return std::inner_product(__first1, __last1, __first2, __init, __binary_op1, __binary_op2);
+    return std::inner_product(__first1, __last1, __first2, std::move(__init), __binary_op1, __binary_op2);
 }
 
 template <class _RandomAccessIterator1, class _RandomAccessIterator2, class _Tp, class _BinaryOperation1,
@@ -48,7 +48,7 @@ __brick_transform_reduce(_RandomAccessIterator1 __first1, _RandomAccessIterator1
 {
     typedef typename std::iterator_traits<_RandomAccessIterator1>::difference_type _DifferenceType;
     return __unseq_backend::__simd_transform_reduce(
-        __last1 - __first1, __init, __binary_op1,
+        __last1 - __first1, std::move(__init), __binary_op1,
         [=, &__binary_op2](_DifferenceType __i) { return __binary_op2(__first1[__i], __first2[__i]); });
 }
 
@@ -59,7 +59,7 @@ __pattern_transform_reduce(_Tag, _ExecutionPolicy&&, _ForwardIterator1 __first1,
                            _ForwardIterator2 __first2, _Tp __init, _BinaryOperation1 __binary_op1,
                            _BinaryOperation2 __binary_op2) noexcept
 {
-    return __brick_transform_reduce(__first1, __last1, __first2, __init, __binary_op1, __binary_op2,
+    return __brick_transform_reduce(__first1, __last1, __first2, std::move(__init), __binary_op1, __binary_op2,
                                     typename _Tag::__is_vector{});
 }
 
@@ -79,12 +79,12 @@ __pattern_transform_reduce(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& _
                 __backend_tag{}, std::forward<_ExecutionPolicy>(__exec), __first1, __last1,
                 [__first1, __first2, __binary_op2](_RandomAccessIterator1 __i) mutable
                 { return __binary_op2(*__i, *(__first2 + (__i - __first1))); },
-                __init,
+                std::move(__init),
                 __binary_op1, // Combine
                 [__first1, __first2, __binary_op1, __binary_op2](_RandomAccessIterator1 __i, _RandomAccessIterator1 __j,
                                                                  _Tp __init) -> _Tp
                 {
-                    return __internal::__brick_transform_reduce(__i, __j, __first2 + (__i - __first1), __init,
+                    return __internal::__brick_transform_reduce(__i, __j, __first2 + (__i - __first1), std::move(__init),
                                                                 __binary_op1, __binary_op2, _IsVector{});
                 });
         });
@@ -99,7 +99,7 @@ _Tp
 __brick_transform_reduce(_ForwardIterator __first, _ForwardIterator __last, _Tp __init, _BinaryOperation __binary_op,
                          _UnaryOperation __unary_op, /*is_vector=*/std::false_type) noexcept
 {
-    return std::transform_reduce(__first, __last, __init, __binary_op, __unary_op);
+    return std::transform_reduce(__first, __last, std::move(__init), __binary_op, __unary_op);
 }
 
 template <class _RandomAccessIterator, class _Tp, class _UnaryOperation, class _BinaryOperation>
@@ -110,7 +110,7 @@ __brick_transform_reduce(_RandomAccessIterator __first, _RandomAccessIterator __
 {
     typedef typename std::iterator_traits<_RandomAccessIterator>::difference_type _DifferenceType;
     return __unseq_backend::__simd_transform_reduce(
-        __last - __first, __init, __binary_op,
+        __last - __first, std::move(__init), __binary_op,
         [=, &__unary_op](_DifferenceType __i) { return __unary_op(__first[__i]); });
 }
 
@@ -120,7 +120,7 @@ _Tp
 __pattern_transform_reduce(_Tag, _ExecutionPolicy&&, _ForwardIterator __first, _ForwardIterator __last, _Tp __init,
                            _BinaryOperation __binary_op, _UnaryOperation __unary_op) noexcept
 {
-    return __internal::__brick_transform_reduce(__first, __last, __init, __binary_op, __unary_op,
+    return __internal::__brick_transform_reduce(__first, __last, std::move(__init), __binary_op, __unary_op,
                                                 typename _Tag::__is_vector{});
 }
 
@@ -138,9 +138,9 @@ __pattern_transform_reduce(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& _
         {
             return __par_backend::__parallel_transform_reduce(
                 __backend_tag{}, std::forward<_ExecutionPolicy>(__exec), __first, __last,
-                [__unary_op](_RandomAccessIterator __i) mutable { return __unary_op(*__i); }, __init, __binary_op,
+                [__unary_op](_RandomAccessIterator __i) mutable { return __unary_op(*__i); }, std::move(__init), __binary_op,
                 [__unary_op, __binary_op](_RandomAccessIterator __i, _RandomAccessIterator __j, _Tp __init) {
-                    return __internal::__brick_transform_reduce(__i, __j, __init, __binary_op, __unary_op, _IsVector{});
+                    return __internal::__brick_transform_reduce(__i, __j, std::move(__init), __binary_op, __unary_op, _IsVector{});
                 });
         });
 }
@@ -181,7 +181,7 @@ __brick_transform_scan(_RandomAccessIterator __first, _RandomAccessIterator __la
         __init = __binary_op(__init, __unary_op(*__first));
         *__result = __init;
     }
-    return std::make_pair(__result, __init);
+    return std::make_pair(__result, std::move(__init));
 }
 
 // type is arithmetic and binary operation is a user defined operation.
@@ -199,11 +199,11 @@ __brick_transform_scan(_RandomAccessIterator __first, _RandomAccessIterator __la
                        /*is_vector=*/std::true_type) noexcept
 {
 #if defined(_PSTL_UDS_PRESENT)
-    return __unseq_backend::__simd_scan(__first, __last - __first, __result, __unary_op, __init, __binary_op,
+    return __unseq_backend::__simd_scan(__first, __last - __first, __result, __unary_op, std::move(__init), __binary_op,
                                         _Inclusive());
 #else
     // We need to call serial brick here to call function for inclusive and exclusive scan that depends on _Inclusive() value
-    return __internal::__brick_transform_scan(__first, __last, __result, __unary_op, __init, __binary_op, _Inclusive(),
+    return __internal::__brick_transform_scan(__first, __last, __result, __unary_op, std::move(__init), __binary_op, _Inclusive(),
                                               /*is_vector=*/std::false_type());
 #endif
 }
@@ -215,7 +215,7 @@ __brick_transform_scan(_RandomAccessIterator __first, _RandomAccessIterator __la
                        _UnaryOperation __unary_op, _Tp __init, _BinaryOperation __binary_op, _Inclusive,
                        /*is_vector=*/std::true_type) noexcept
 {
-    return __internal::__brick_transform_scan(__first, __last, __result, __unary_op, __init, __binary_op, _Inclusive(),
+    return __internal::__brick_transform_scan(__first, __last, __result, __unary_op, std::move(__init), __binary_op, _Inclusive(),
                                               /*is_vector=*/std::false_type());
 }
 
@@ -247,19 +247,19 @@ __pattern_transform_scan(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __e
         {
             __par_backend::__parallel_transform_scan(
                 __backend_tag{}, std::forward<_ExecutionPolicy>(__exec), __last - __first,
-                [__first, __unary_op](_DifferenceType __i) mutable { return __unary_op(__first[__i]); }, __init,
+                [__first, __unary_op](_DifferenceType __i) mutable { return __unary_op(__first[__i]); }, std::move(__init),
                 __binary_op,
                 [__first, __unary_op, __binary_op](_DifferenceType __i, _DifferenceType __j, _Tp __init)
                 {
                     // Execute serial __brick_transform_reduce, due to the explicit SIMD vectorization (reduction) requires a commutative operation for the guarantee of correct scan.
-                    return __internal::__brick_transform_reduce(__first + __i, __first + __j, __init, __binary_op,
+                    return __internal::__brick_transform_reduce(__first + __i, __first + __j, std::move(__init), __binary_op,
                                                                 __unary_op,
                                                                 /*__is_vector*/ std::false_type());
                 },
                 [__first, __unary_op, __binary_op, __result](_DifferenceType __i, _DifferenceType __j, _Tp __init)
                 {
                     return __internal::__brick_transform_scan(__first + __i, __first + __j, __result + __i, __unary_op,
-                                                              __init, __binary_op, _Inclusive(), _IsVector{})
+                                                              std::move(__init), __binary_op, _Inclusive(), _IsVector{})
                         .second;
                 });
             return __result + (__last - __first);
@@ -286,7 +286,7 @@ __pattern_transform_scan(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __e
         [&]()
         {
             __par_backend::__parallel_strict_scan(
-                __backend_tag{}, std::forward<_ExecutionPolicy>(__exec), __n, __init,
+                __backend_tag{}, std::forward<_ExecutionPolicy>(__exec), __n, std::move(__init),
                 [__first, __unary_op, __binary_op, __result](_DifferenceType __i, _DifferenceType __len)
                 {
                     return __internal::__brick_transform_scan(__first + __i, __first + (__i + __len), __result + __i,

diff --git a/libstdc++-v3/include/std/numeric b/libstdc++-v3/include/std/numeric
index 490963ee46d1d9d580cd1d0bd32f606b931fc486..cbabf031216e5ac43245475b7c43ee1adc5e8698 100644 (file)
--- a/libstdc++-v3/include/std/numeric
+++ b/libstdc++-v3/include/std/numeric
@@ -582,7 +582,7 @@ namespace __detail
     {
       if (__first != __last)
        {
-         auto __init = *__first;
+         auto __init = std::move(*__first);
          *__result++ = __init;
          ++__first;
          if (__first != __last)
@@ -645,8 +645,8 @@ namespace __detail
     {
       while (__first != __last)
        {
-         auto __v = __init;
-         __init = __binary_op(__init, __unary_op(*__first));
+         auto __v = std::move(__init);
+         __init = __binary_op(__v, __unary_op(*__first));
          ++__first;
          *__result++ = std::move(__v);
        }

diff --git a/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/108236.cc b/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/108236.cc
index e0e3027b321876f52241f41b10d1d5da18426d63..cbef8ab67dde8f34d751315c0a8cea3f9a82a6a1 100644 (file)
--- a/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/108236.cc
+++ b/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/108236.cc
@@ -8,30 +8,6 @@
 #include <execution>
 #include <testsuite_hooks.h>
 
-struct Mint
-{
-  Mint(int i = 0) : val(i) { }
-  Mint(Mint&&) = default;
-  Mint& operator=(Mint&&) = default;
-
-  operator int() const { return val; }
-
-private:
-  int val;
-};
-
-void
-test_move_only()
-{
-  const int input[]{10, 20, 30};
-  int output[3];
-  std::exclusive_scan(std::execution::seq, input, input+3, output, Mint(5),
-                     std::plus<int>{});
-  VERIFY( output[0] == 5 );
-  VERIFY( output[1] == 15 );
-  VERIFY( output[2] == 35 );
-}
-
 void
 test_pr108236()
 {
@@ -45,6 +21,5 @@ test_pr108236()
 
 int main()
 {
-  test_move_only();
   test_pr108236();
 }

diff --git a/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/move_only.cc b/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/move_only.cc
new file mode 100644 (file)
index 0000000..38ad3c2
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/move_only.cc
@@ -0,0 +1,90 @@
+// { dg-options "-ltbb" }
+// { dg-do run { target c++17 } }
+// { dg-require-effective-target tbb_backend }
+
+#include <numeric>
+#include <execution>
+#include <testsuite_hooks.h>
+
+struct Mint
+{
+  Mint(int i = 0) : val(i) { }
+
+  Mint(Mint&& m) : val(m.val) { m.val = -1; }
+
+  Mint& operator=(Mint&& m)
+  {
+    val = m.val;
+    m.val = -1;
+    return *this;
+  }
+
+  operator int() const
+  {
+    VERIFY(val >= 0); // Check we don't read value of a moved-from instance.
+    return val;
+  }
+
+  friend Mint operator+(const Mint& lhs, const Mint& rhs)
+  { return Mint(lhs.val + rhs.val); }
+
+private:
+  int val;
+};
+
+void
+test_reduce()
+{
+  Mint input[]{1, 2, 3};
+  Mint m = std::reduce(std::execution::seq, input, input+3);
+  VERIFY( static_cast<int>(m) == 6 );
+
+  m = std::reduce(std::execution::seq, input, input+3, Mint(100));
+  VERIFY( static_cast<int>(m) == 106 );
+
+  m = std::reduce(std::execution::seq, input, input+3, Mint(200),
+                 std::plus<>{});
+  VERIFY( static_cast<int>(m) == 206 );
+}
+
+void
+test_transform_reduce()
+{
+}
+
+void
+test_exclusive_scan()
+{
+  const int input[]{10, 20, 30};
+  int output[3];
+  std::exclusive_scan(std::execution::seq, input, input+3, output, Mint(5),
+                     std::plus<int>{});
+  VERIFY( output[0] == 5 );
+  VERIFY( output[1] == 15 );
+  VERIFY( output[2] == 35 );
+}
+
+void
+test_inclusive_scan()
+{
+}
+
+void
+test_transform_exclusive_scan()
+{
+}
+
+void
+test_transform_inclusive_scan()
+{
+}
+
+int main()
+{
+  test_reduce();
+  test_transform_reduce();
+  test_exclusive_scan();
+  test_inclusive_scan();
+  test_transform_exclusive_scan();
+  test_transform_inclusive_scan();
+}