[PATCH] Replace direct PSTL uses of assert() with a macro
Jonathan Wakely
jwakely@redhat.com
Wed Apr 10 20:01:00 GMT 2019
On 09/04/19 15:23 -0700, Thomas Rodgers wrote:
>This also replaces calls to __TBB_ASSERT so that there are two macro
>definitions provided by c++config -
> __PSTL_ASSERT(_Condition)
> __PSTL_ASSERT_MSG(_Condition, _Message)
>
> * include/bits/c++config:
> Add definition for __PSTL_ASSERT.
> Add definition for __PSTL_ASSERT_MSG.
> * include/pstl/algorithm_impl.h: Replace use of assert().
> * include/pstl/numeric_impl.h: Replace use of assert().
> * include/pstl/parallel_backend_tbb.h:
> Replace use of assert().
> Replace use of __TBB_ASSERT().
>
> * include/pstl/parallel_backend_utils.h: Replace use of assert().
>
>From d95934a0f325e0934ada829378c3c0dfd6b3628c Mon Sep 17 00:00:00 2001
>From: Thomas Rodgers <trodgers@redhat.com>
>Date: Fri, 5 Apr 2019 15:27:35 -0700
>Subject: [PATCH] Replace direct PSTL uses of assert() with a macro
>
>This also replaces calls to __TBB_ASSERT so that there are two macro
>definitions provided by c++config -
> __PSTL_ASSERT(_Condition)
> __PSTL_ASSERT_MSG(_Condition, _Message)
>
> * include/bits/c++config:
> Add definition for __PSTL_ASSERT.
> Add definition for __PSTL_ASSERT_MSG.
> * include/pstl/algorithm_impl.h: Replace use of assert().
> * include/pstl/numeric_impl.h: Replace use of assert().
> * include/pstl/parallel_backend_tbb.h:
> Replace use of assert().
> Replace use of __TBB_ASSERT().
>
> * include/pstl/parallel_backend_utils.h: Replace use of assert().
>---
> libstdc++-v3/include/bits/c++config | 4 ++++
> libstdc++-v3/include/pstl/algorithm_impl.h | 14 +++++++-------
> libstdc++-v3/include/pstl/numeric_impl.h | 8 ++++----
> libstdc++-v3/include/pstl/parallel_backend_tbb.h | 11 ++++++-----
> .../include/pstl/parallel_backend_utils.h | 15 +++++++--------
> 5 files changed, 28 insertions(+), 24 deletions(-)
>
>diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
>index 66420a9a3f2..8dd04f218b4 100644
>--- a/libstdc++-v3/include/bits/c++config
>+++ b/libstdc++-v3/include/bits/c++config
>@@ -690,6 +690,10 @@ namespace std
> # undef __PSTL_PAR_BACKEND_TBB
> # endif
>
>+# define __PSTL_ASSERT(_Condition) (__glibcxx_assert(_Condition))
>+# define __PSTL_ASSERT_MSG(_Condition, _Message) (__glibcxx_assert(_Condition))
I don't think these extra parens around the assert will work. When
_GLIBXCXX_ASSERTIONS is defined __glibcxx_assert expands to a do-while
statement, which isn't valid inside an (expression).
> # define __PSTL_PRAGMA(x) _Pragma (#x)
>
> # define __PSTL_STRING_AUX(x) #x
>diff --git a/libstdc++-v3/include/pstl/algorithm_impl.h b/libstdc++-v3/include/pstl/algorithm_impl.h
>index e06bf60151e..a42d3993d1b 100644
>--- a/libstdc++-v3/include/pstl/algorithm_impl.h
>+++ b/libstdc++-v3/include/pstl/algorithm_impl.h
>@@ -1309,7 +1309,7 @@ __pattern_unique_copy(_ExecutionPolicy&& __exec, _RandomAccessIterator __first,
> return __internal::__except_handler([&__exec, __n, __first, __result, __pred, __is_vector, &__mask_buf]() {
> bool* __mask = __mask_buf.get();
> _DifferenceType __m{};
>- __par_backend::parallel_strict_scan(
>+ __par_backend::__parallel_strict_scan(
> std::forward<_ExecutionPolicy>(__exec), __n, _DifferenceType(0),
> [=](_DifferenceType __i, _DifferenceType __len) -> _DifferenceType { // Reduce
> _DifferenceType __extra = 0;
>@@ -2731,8 +2731,8 @@ __pattern_includes(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _Forwa
> return !__internal::__parallel_or(
> std::forward<_ExecutionPolicy>(__exec), __first2, __last2,
> [__first1, __last1, __first2, __last2, &__comp](_ForwardIterator2 __i, _ForwardIterator2 __j) {
>- assert(__j > __i);
>- //assert(__j - __i > 1);
>+ __PSTL_ASSERT(__j > __i);
>+ //__PSTL_ASSERT(__j - __i > 1);
>
> //1. moving boundaries to "consume" subsequence of equal elements
> auto __is_equal = [&__comp](_ForwardIterator2 __a, _ForwardIterator2 __b) -> bool {
>@@ -2756,8 +2756,8 @@ __pattern_includes(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _Forwa
> //2. testing is __a subsequence of the second range included into the first range
> auto __b = std::lower_bound(__first1, __last1, *__i, __comp);
>
>- assert(!__comp(*(__last1 - 1), *__b));
>- assert(!__comp(*(__j - 1), *__i));
>+ __PSTL_ASSERT(!__comp(*(__last1 - 1), *__b));
>+ __PSTL_ASSERT(!__comp(*(__j - 1), *__i));
> return !std::includes(__b, __last1, __i, __j, __comp);
> });
> });
>@@ -2801,7 +2801,7 @@ __parallel_set_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _Forwar
> __internal::__brick_move(__buffer + __s.__buf_pos, __buffer + (__s.__buf_pos + __s.__len), __result + __s.__pos,
> __is_vector);
> };
>- __par_backend::parallel_strict_scan(
>+ __par_backend::__parallel_strict_scan(
This patch seems to include some additional fixes that aren't
mentioned in the changelog.
> std::forward<_ExecutionPolicy>(__exec), __n1, _SetRange{0, 0, 0}, //-1, 0},
> [=](_DifferenceType __i, _DifferenceType __len) { // Reduce
> //[__b; __e) - a subrange of the first sequence, to reduce
>@@ -2948,7 +2948,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _
> }
>
> const auto __m2 = __left_bound_seq_2 - __first2;
>- assert(__m1 == 0 || __m2 == 0);
>+ __PSTL_ASSERT(__m1 == 0 || __m2 == 0);
> if (__m2 > __set_algo_cut_off)
> {
> auto __res_or = __result;
>diff --git a/libstdc++-v3/include/pstl/numeric_impl.h b/libstdc++-v3/include/pstl/numeric_impl.h
>index 49a4abf5a95..86f8ddf36a3 100644
>--- a/libstdc++-v3/include/pstl/numeric_impl.h
>+++ b/libstdc++-v3/include/pstl/numeric_impl.h
>@@ -278,8 +278,8 @@ __pattern_transform_scan(_ExecutionPolicy&& __exec, _RandomAccessIterator __firs
> __par_backend::parallel_strict_scan(
> std::forward<_ExecutionPolicy>(__exec), __n, __init,
> [__first, __unary_op, __binary_op, __result, __is_vector](_DifferenceType __i, _DifferenceType __len) {
>- return __internal::__brick_transform_scan(__first + __i, __first + (__i + __len), __result + __i, __unary_op, _Tp{},
>- __binary_op, _Inclusive(), __is_vector)
>+ return __internal::__brick_transform_scan(__first + __i, __first + (__i + __len), __result + __i,
>+ __unary_op, _Tp{}, __binary_op, _Inclusive(), __is_vector)
Here too.
> .second;
> },
> __binary_op,
>@@ -314,7 +314,7 @@ _ForwardIterator2
> __brick_adjacent_difference(_ForwardIterator1 __first, _ForwardIterator1 __last, _ForwardIterator2 __d_first,
> BinaryOperation __op, /*is_vector=*/std::true_type) noexcept
> {
>- assert(__first != __last);
>+ __PSTL_ASSERT(__first != __last);
>
> typedef typename std::iterator_traits<_ForwardIterator1>::reference _ReferenceType1;
> typedef typename std::iterator_traits<_ForwardIterator2>::reference _ReferenceType2;
>@@ -344,7 +344,7 @@ __pattern_adjacent_difference(_ExecutionPolicy&& __exec, _ForwardIterator1 __fir
> _ForwardIterator2 __d_first, _BinaryOperation __op, _IsVector __is_vector,
> /*is_parallel=*/std::true_type)
> {
>- assert(__first != __last);
>+ __PSTL_ASSERT(__first != __last);
> typedef typename std::iterator_traits<_ForwardIterator1>::reference _ReferenceType1;
> typedef typename std::iterator_traits<_ForwardIterator2>::reference _ReferenceType2;
>
>diff --git a/libstdc++-v3/include/pstl/parallel_backend_tbb.h b/libstdc++-v3/include/pstl/parallel_backend_tbb.h
>index f09f47a8a89..8195684048b 100644
>--- a/libstdc++-v3/include/pstl/parallel_backend_tbb.h
>+++ b/libstdc++-v3/include/pstl/parallel_backend_tbb.h
>@@ -139,7 +139,7 @@ struct __par_trans_red_body
> _Tp&
> sum()
> {
>- __TBB_ASSERT(_M_has_sum, "sum expected");
>+ __PSTL_ASSERT_MSG(_M_has_sum, "sum expected");
> return *(_Tp*)_M_sum_storage;
> }
> __par_trans_red_body(_Up __u, _Tp __init, _Cp __c, _Rp __r)
>@@ -173,7 +173,7 @@ struct __par_trans_red_body
> _Index __j = __range.end();
> if (!_M_has_sum)
> {
>- __TBB_ASSERT(__range.size() > 1, "there should be at least 2 elements");
>+ __PSTL_ASSERT_MSG(__range.size() > 1, "there should be at least 2 elements");
> new (&_M_sum_storage)
> _Tp(_M_combine(_M_u(__i), _M_u(__i + 1))); // The condition i+1 < j is provided by the grain size of 3
> _M_has_sum = true;
>@@ -233,7 +233,7 @@ class __trans_scan_body
> _Tp&
> sum() const
> {
>- __TBB_ASSERT(_M_has_sum, "sum expected");
>+ __PSTL_ASSERT_MSG(_M_has_sum, "sum expected");
> return *const_cast<_Tp*>(reinterpret_cast<_Tp const*>(_M_sum_storage));
> }
>
>@@ -347,7 +347,8 @@ __downsweep(_Index __i, _Index __m, _Index __tilesize, _Tp* __r, _Index __lastsi
> // T must have a trivial constructor and destructor.
> template <class _ExecutionPolicy, typename _Index, typename _Tp, typename _Rp, typename _Cp, typename _Sp, typename _Ap>
> void
>-parallel_strict_scan(_ExecutionPolicy&&, _Index __n, _Tp __initial, _Rp __reduce, _Cp __combine, _Sp __scan, _Ap __apex)
>+__parallel_strict_scan(_ExecutionPolicy&&, _Index __n, _Tp __initial, _Rp __reduce, _Cp __combine, _Sp __scan,
>+ _Ap __apex)
And here.
The assert parts look fine, and this uglification looks fine, but they
should be in separate patches please.
More information about the Libstdc++
mailing list