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.
This commit is contained in:
Jonathan Wakely
2024-12-04 21:50:22 +00:00
committed by Jonathan Wakely
parent a067cbcdcc
commit df1d436d17
5 changed files with 119 additions and 54 deletions

View File

@@ -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());
}

View File

@@ -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,

View File

@@ -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);
}

View File

@@ -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();
}

View File

@@ -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();
}