[pstl] Replace direct use of assert() with __PSTL_ASSERT

[ldionne]

Summary

Standard libraries may (libstdc++ in particular) forbid direct use of
assert()/<cassert> in library code.

[ldionne]

pstl/include/pstl/internal/pstl_config.h

17–28

I would do this:

#if !defined(_PSTL_ASSERT)
#  include <cassert>
#  define _PSTL_ASSERT(x) assert(x)
#endif

And I would just use _PSTL_ASSERT(x) elsewhere in the code. I don't see a reason to have a separate _PSTL_USE_ASSERT macro if we can define _PSTL_ASSERT directly. Also, whether NDEBUG is defined will control whether _PSTL_ASSERT does something by default, so the default is fine for unit tests. WDYT?

[rodgert]

pstl/include/pstl/internal/pstl_config.h

17–28

That works for me. I will be injecting the definitions for PSTL_ASSERT() and PSTL_ASSERT_MSG() from libstdc++'s configuration anyway.

This is really just to try and provide a sane default the standalone impl. I'll update the diff.

[rodgert]

• [pstl] Supply definition of __PSTL_ASSERT() only if not already defined

[ldionne]

LGTM, except for the nit.

pstl/test/CMakeLists.txt

31

Not needed anymore.

[ldionne]

Rebased onto master

[rodgert]

I’ll make sure both sides agree on the name. I still need to go through the
tests on the libstdc++ side but that can wait until after stage1 closes.

pstl/include/pstl/internal/algorithm_impl.h

__pattern_includes(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _ForwardIterator1 __last1,
 
     if (__last2 - __first2 == 1)
         return !__comp(*__first1, *__first2) && !__comp(*__first2, *__first1);
 
     return __internal::__except_handler([&]() {
         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 {
                     return !__comp(*__a, *__b) && !__comp(*__b, *__a);
                 };
 
                 //1.1 left bound, case "aaa[aaaxyz...]" - searching "x"
                 if (__i > __first2 && __is_equal(__i, __i - 1))
                 {
                     //whole subrange continues to content equal elements - return "no op"
                     if (__is_equal(__i, __j - 1))
                         return false;
 
                     __i = std::upper_bound(__i, __last2, *__i, __comp);
                 }
 
                 //1.2 right bound, case "[...aaa]aaaxyz" - searching "x"
                 if (__j < __last2 && __is_equal(__j - 1, __j))
                     __j = std::upper_bound(__j, __last2, *__j, __comp);
 
                 //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);
             });
     });
 }
 #endif
 
 constexpr auto __set_algo_cut_off = 1000;
 
if (__m1 > __set_algo_cut_off)
                     std::forward<_ExecutionPolicy>(__exec), __left_bound_seq_1, __last1, __first2, __last2, __result,
                     __comp, [](_DifferenceType __n, _DifferenceType __m) { return __n + __m; }, __set_union_op,
                     __is_vector);
             });
         return __result;
     }
 
     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;
         __result += __m2; //we know proper offset due to [first2; left_bound_seq_2) < [first1; last1)
         __par_backend::__parallel_invoke(
             std::forward<_ExecutionPolicy>(__exec),
             //do parallel copying of [first2; left_bound_seq_2)
             [=] {

pstl/include/pstl/internal/numeric_impl.h

__brick_adjacent_difference(_ForwardIterator __first, _ForwardIterator __last, _OutputIterator __d_first,
     return std::adjacent_difference(__first, __last, __d_first, __op);
 }
 
 template <class _ForwardIterator1, class _ForwardIterator2, class BinaryOperation>
 _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;
 
     auto __n = __last - __first;
     *__d_first = *__first;
     return __unseq_backend::__simd_walk_3(
         __first + 1, __n - 1, __first, __d_first + 1,

 #if __PSTL_USE_PAR_POLICIES
 template <class _ExecutionPolicy, class _ForwardIterator1, class _ForwardIterator2, class _BinaryOperation,
           class _IsVector>
 _ForwardIterator2
 __pattern_adjacent_difference(_ExecutionPolicy&& __exec, _ForwardIterator1 __first, _ForwardIterator1 __last,
                               _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;
 
     *__d_first = *__first;
     __par_backend::__parallel_for(
         std::forward<_ExecutionPolicy>(__exec), __first, __last - 1,
         [&__op, __is_vector, __d_first, __first](_ForwardIterator1 __b, _ForwardIterator1 __e) {
             _ForwardIterator2 __d_b = __d_first + (__b - __first);

pstl/include/pstl/internal/parallel_backend_tbb.h

struct __par_trans_red_body
     alignas(_Tp) char _M_sum_storage[sizeof(_Tp)]; // Holds generalized non-commutative sum when has_sum==true
     _Rp _M_brick_reduce;                           // Most likely to have non-empty layout
     _Up _M_u;
     _Cp _M_combine;
     bool _M_has_sum; // Put last to minimize size of class
     _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)
         : _M_brick_reduce(__r), _M_u(__u), _M_combine(__c), _M_has_sum(true)
     {
         new (_M_sum_storage) _Tp(__init);
     }
 
struct __par_trans_red_body
 
     void
     operator()(const tbb::blocked_range<_Index>& __range)
     {
         _Index __i = __range.begin();
         _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;
             std::advance(__i, 2);
             if (__i == __j)
                 return;
         }
         sum() = _M_brick_reduce(__i, __j, sum());
~__trans_scan_body()
         // 17.6.5.12 tells us to not worry about catching exceptions from destructors.
         if (_M_has_sum)
             sum().~_Tp();
     }
 
     _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));
     }
 
     void
     operator()(const tbb::blocked_range<_Index>& __range, tbb::pre_scan_tag)
     {
         _Index __i = __range.begin();
         _Index __j = __range.end();
tbb::this_task_arena::isolate([=, &__nsort]() {
         typedef typename std::iterator_traits<_RandomAccessIterator>::difference_type _DifferenceType;
         const _DifferenceType __n = __xe - __xs;
         if (__nsort == 0)
             __nsort = __n;
 
         const _DifferenceType __sort_cut_off = __PSTL_STABLE_SORT_CUT_OFF;
         if (__n > __sort_cut_off)
         {
-            assert(__nsort > 0 && __nsort <= __n);
+            __PSTL_ASSERT(__nsort > 0 && __nsort <= __n);
             __buffer<_ValueType> __buf(__n);
             using tbb::task;
             task::spawn_root_and_wait(*new (task::allocate_root())
                                           __stable_sort_task<_RandomAccessIterator, _ValueType*, _Compare, _LeafSort>(
                                               __xs, __xe, (_ValueType*)__buf.get(), 2, __comp, __leaf_sort, __nsort));
             return;
         }
         //serial sort

pstl/include/pstl/internal/parallel_backend_utils.h

 // -*- C++ -*-
 //===-- parallel_backend_utils.h ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef __PSTL_parallel_backend_utils_H
 #define __PSTL_parallel_backend_utils_H
 
 #include <iterator>
 #include <utility>
-#include <cassert>
 #include "utils.h"
 
 namespace __pstl
 {
 namespace __par_backend
 {
 
 //! Destroy sequence [xs,xe)
struct __serial_move_merge
     {
     }
     template <class _RandomAccessIterator1, class _RandomAccessIterator2, class _RandomAccessIterator3, class _Compare>
     void
     operator()(_RandomAccessIterator1 __xs, _RandomAccessIterator1 __xe, _RandomAccessIterator2 __ys,
                _RandomAccessIterator2 __ye, _RandomAccessIterator3 __zs, _Compare __comp)
     {
         auto __n = _M_nmerge;
-        assert(__n > 0);
+        __PSTL_ASSERT(__n > 0);
         if (__xs != __xe)
         {
             if (__ys != __ye)
             {
                 for (;;)
                 {
                     if (__comp(*__ys, *__xs))
                     {
class __stack
     void
     operator=(const __stack&) = delete;
 
   public:
     __stack(_DifferenceType __max_size) : _M_buf(__max_size), _M_maxsize(__max_size) { _M_ptr = _M_buf.get(); }
 
     ~__stack()
     {
-        assert(size() <= _M_maxsize);
+        __PSTL_ASSERT(size() <= _M_maxsize);
         while (!empty())
             pop();
     }
 
     const _Buf&
     buffer() const
     {
         return _M_buf;
     }
     size_t
     size() const
     {
-        assert(_M_ptr - _M_buf.get() <= _M_maxsize);
-        assert(_M_ptr - _M_buf.get() >= 0);
+        __PSTL_ASSERT(_M_ptr - _M_buf.get() <= _M_maxsize);
+        __PSTL_ASSERT(_M_ptr - _M_buf.get() >= 0);
         return _M_ptr - _M_buf.get();
     }
     bool
     empty() const
     {
-        assert(_M_ptr >= _M_buf.get());
+        __PSTL_ASSERT(_M_ptr >= _M_buf.get());
         return _M_ptr == _M_buf.get();
     }
     void
     push(const _ValueType& __v)
     {
-        assert(size() < _M_maxsize);
+        __PSTL_ASSERT(size() < _M_maxsize);
         new (_M_ptr) _ValueType(__v);
         ++_M_ptr;
     }
     const _ValueType&
     top() const
     {
         return *(_M_ptr - 1);
     }
     void
     pop()
     {
-        assert(_M_ptr > _M_buf.get());
+        __PSTL_ASSERT(_M_ptr > _M_buf.get());
         --_M_ptr;
         (*_M_ptr).~_ValueType();
     }
 };
 
 } // namespace __par_backend
 } // namespace __pstl
 
 #endif /* __PSTL_parallel_backend_utils_H */

pstl/include/pstl/internal/pstl_config.h

 // -*- C++ -*-
 //===-- pstl_config.h -----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef __PSTL_config_H
 #define __PSTL_config_H
 
 #define PSTL_VERSION 203
 #define PSTL_VERSION_MAJOR (PSTL_VERSION / 100)
 #define PSTL_VERSION_MINOR (PSTL_VERSION - PSTL_VERSION_MAJOR * 100)
 
+#if defined(PSTL_USE_ASSERT)
+#    undef __PSTL_USE_ASSERT
+#    define __PSTL_USE_ASSERT PSTL_USE_ASSERT
+#    include <cassert>
+#    define __PSTL_ASSERT(pred) (assert((pred)))
+// TODO make this smarter
+#    define __PSTL_ASSERT_MSG(pred, msg) (assert((pred)))
+#else
+#    undef __PSTL_USE_ASSERT
+#    define __PSTL_ASSERT(pred) ((void)0)
+#    define __PSTL_ASSERT_MSG(pred, msg) ((void)0)
+#endif
+
 // Check the user-defined macro for parallel policies
 #if defined(PSTL_USE_PARALLEL_POLICIES)
 #    undef __PSTL_USE_PAR_POLICIES
 #    define __PSTL_USE_PAR_POLICIES PSTL_USE_PARALLEL_POLICIES
 // Check the internal macro for parallel policies
 #elif !defined(__PSTL_USE_PAR_POLICIES)
 #    define __PSTL_USE_PAR_POLICIES 1
 #endif

pstl/test/CMakeLists.txt

foreach(_file IN LISTS UNIT_TESTS)
     file(RELATIVE_PATH _target "${CMAKE_CURRENT_SOURCE_DIR}" "${_file}")
     string(REPLACE ".cpp" "" _target "${_target}")
     string(REPLACE "/" "-" _target "${_target}")
     set(_target "pstl-${_target}")
 
     add_executable(${_target} EXCLUDE_FROM_ALL "${_file}")
     target_include_directories(${_target} PRIVATE "${CMAKE_CURRENT_LIST_DIR}")
     target_link_libraries(${_target} PRIVATE pstl::ParallelSTL)
-    target_compile_definitions(${_target} PRIVATE -DPSTL_STANDALONE_TESTS)
+    target_compile_definitions(${_target} PRIVATE -DPSTL_STANDALONE_TESTS -DPSTL_USE_ASSERT)
     set_target_properties(${_target} PROPERTIES CXX_EXTENSIONS NO
                                                 RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}")
     add_test(${_target} "${CMAKE_CURRENT_BINARY_DIR}/${_target}")
     add_dependencies(pstl-build-tests ${_target})
 endforeach()