On Tue, 29 Apr 2025 at 09:45, Jonathan Wakely <[email protected]> wrote:
>
> 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.
> ---
>
> Tested x86_64-linux.
Pushed to trunk now. This should be backported too.
>
> libstdc++-v3/include/pstl/glue_numeric_impl.h | 16 ++--
> libstdc++-v3/include/pstl/numeric_impl.h | 36 ++++----
> libstdc++-v3/include/std/numeric | 6 +-
> .../26_numerics/pstl/numeric_ops/108236.cc | 25 ------
> .../26_numerics/pstl/numeric_ops/move_only.cc | 90 +++++++++++++++++++
> 5 files changed, 119 insertions(+), 54 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/26_numerics/pstl/numeric_ops/move_only.cc
>
> diff --git a/libstdc++-v3/include/pstl/glue_numeric_impl.h
> b/libstdc++-v3/include/pstl/glue_numeric_impl.h
> index 10d4912deed..fe2d0fd47e2 100644
> --- 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 e1ebec16039..b285a667653 100644
> --- 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 490963ee46d..cbabf031216 100644
> --- 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 e0e3027b321..cbef8ab67dd 100644
> --- 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
> index 00000000000..38ad3c2877e
> --- /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();
> +}
> --
> 2.49.0
>
