[libc++][PSTL] Implement std::sort

[philnik]

Try to fix CI

[ldionne]

libcxx/include/__algorithm/pstl_sort.h

32–38

This way of defining the "two overloads" (the one with and the one without a comp) will accept the following code, which shouldn't be valid:

std::sort(policy, it1, it2, {});

Instead, let's do like we do in the serial std::sort and have two separate overloads (and make the no-comp overload simply call the comp overload with std::less<>()).

Let's also add a SFINAE test to ensure that we don't accept that code (and while we're at it let's add one to the serial std::sort if we don't have one already).

41–42

Is there any reason we're not std::moveing the arguments like __g_comp into std::stable_sort? The same observation probably applies to all the other algorithms we have so far.

libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/pstl.sort.pass.cpp

37

I don't think you have any tests for the algorithm with a custom comparator.

libcxx/test/support/test_macros.h

278

Let's move this closer to line 366 where we define the other similar macros.

[philnik]

Address comments

[philnik]

Try to fix CI

libcxx/docs/Status/PSTLPaper.csv

 | `[alg.rotate] <https://wg21.link/alg.rotate>`_,std::rotate,Nikolas Klauser,|Not Started|
 | `[alg.rotate] <https://wg21.link/alg.rotate>`_,std::rotate_copy,Nikolas Klauser,|Not Started|
 | `[alg.search] <https://wg21.link/alg.search>`_,std::search,Nikolas Klauser,|Not Started|
 | `[alg.search] <https://wg21.link/alg.search>`_,std::search_n,Nikolas Klauser,|Not Started|
 | `[alg.set.operations] <https://wg21.link/alg.set.operations>`_,std::set_difference,Nikolas Klauser,|Not Started|
 | `[alg.set.operations] <https://wg21.link/alg.set.operations>`_,std::set_intersection,Nikolas Klauser,|Not Started|
 | `[alg.set.operations] <https://wg21.link/alg.set.operations>`_,std::set_symmetric_difference,Nikolas Klauser,|Not Started|
 | `[alg.set.operations] <https://wg21.link/alg.set.operations>`_,std::set_union,Nikolas Klauser,|Not Started|
-| `[alg.sort] <https://wg21.link/alg.sort>`_,std::sort,Nikolas Klauser,|Not Started|
+| `[alg.sort] <https://wg21.link/alg.sort>`_,std::sort,Nikolas Klauser,|Complete|
 | `[alg.partitions] <https://wg21.link/alg.partitions>`_,std::stable_partition,Nikolas Klauser,|Not Started|
 | `[alg.sort] <https://wg21.link/alg.sort>`_,std::stable_sort,Nikolas Klauser,|Complete|
 | `[alg.swap] <https://wg21.link/alg.swap>`_,std::swap_ranges,Nikolas Klauser,|Not Started|
 | `[alg.transform] <https://wg21.link/alg.transform>`_,std::transform,Nikolas Klauser,|Complete|
 | `[alg.transform.exclusive.scan] <https://wg21.link/alg.transform.exclusive.scan>`_,std::transform_exclusive_scan,Nikolas Klauser,|Not Started|
 | `[alg.transform.inclusive.scan] <https://wg21.link/alg.transform.inclusive.scan>`_,std::transform_inclusive_scan,Nikolas Klauser,|Not Started|
 | `[transform.reduce] <https://wg21.link/transform.reduce>`_,std::transform_reduce,Nikolas Klauser,|Complete|
 | `[uninitialized.copy] <https://wg21.link/uninitialized.copy>`_,std::uninitialized_copy,Nikolas Klauser,|Not Started|
 | `[uninitialized.copy] <https://wg21.link/uninitialized.copy>`_,std::uninitialized_copy_n,Nikolas Klauser,|Not Started|
 | `[uninitialized.fill] <https://wg21.link/uninitialized.fill>`_,std::uninitialized_fill,Nikolas Klauser,|Not Started|
 | `[uninitialized.fill] <https://wg21.link/uninitialized.fill>`_,std::uninitialized_fill_n,Nikolas Klauser,|Not Started|
 | `[uninitialized.move] <https://wg21.link/uninitialized.move>`_,std::uninitialized_move,Nikolas Klauser,|Not Started|
 | `[uninitialized.move] <https://wg21.link/uninitialized.move>`_,std::uninitialized_move_n,Nikolas Klauser,|Not Started|
 | `[alg.unique] <https://wg21.link/alg.unique>`_,std::unique,Nikolas Klauser,|Not Started|
 | `[alg.unique] <https://wg21.link/alg.unique>`_,std::unique_copy,Nikolas Klauser,|Not Started|

libcxx/include/CMakeLists.txt

set(files
   __algorithm/pstl_fill.h
   __algorithm/pstl_find.h
   __algorithm/pstl_for_each.h
   __algorithm/pstl_frontend_dispatch.h
   __algorithm/pstl_generate.h
   __algorithm/pstl_is_partitioned.h
   __algorithm/pstl_merge.h
   __algorithm/pstl_replace.h
+  __algorithm/pstl_sort.h
   __algorithm/pstl_stable_sort.h
   __algorithm/pstl_transform.h
   __algorithm/push_heap.h
   __algorithm/ranges_adjacent_find.h
   __algorithm/ranges_all_of.h
   __algorithm/ranges_any_of.h
   __algorithm/ranges_binary_search.h
   __algorithm/ranges_clamp.h

libcxx/include/__algorithm/pstl_backend.h

implemented, all the algorithms will eventually forward to the basis algorithms listed above:
   template <class _ExecutionPolicy, class _Iterator, class _OutIterator, class _Pred, class _Tp>
   void __pstl_replace_copy_if(_Backend,
                               _Iterator __first,
                               _Iterator __last,
                               _OutIterator __result,
                               _Pred __pred,
                               const _Tp& __new_value);
 
+  template <class _ExecutionPolicy, class _Iterator, class _Comp>
+  void __pstl_sort(_Backend, _Iterator __first, _Iterator __last, _Comp __comp);
+
 // TODO: Complete this list
 
 */
 
 template <class _ExecutionPolicy>
 struct __select_backend;
 
 template <>

libcxx/include/__algorithm/pstl_sort.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 _LIBCPP___ALGORITHM_PSTL_SORT_H
+#define _LIBCPP___ALGORITHM_PSTL_SORT_H
+
+#include <__algorithm/pstl_backend.h>
+#include <__algorithm/pstl_frontend_dispatch.h>
+#include <__algorithm/pstl_stable_sort.h>
+#include <__config>
+#include <__functional/operations.h>
+#include <__type_traits/is_execution_policy.h>
+#include <__type_traits/remove_cvref.h>
+#include <__utility/forward.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_PSTL) && _LIBCPP_STD_VER >= 17
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class>
+void __pstl_sort();
+
+template <class _ExecutionPolicy,
+          class _RandomAccessIterator,
+          class _Comp,
+          class _RawPolicy                                    = __remove_cvref_t<_ExecutionPolicy>,
+          enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
+_LIBCPP_HIDE_FROM_ABI void
+sort(_ExecutionPolicy&& __policy, _RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) {
+  std::__pstl_frontend_dispatch(
+      _LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_sort),
+      [&__policy](_RandomAccessIterator __g_first, _RandomAccessIterator __g_last, _Comp __g_comp) {
+        std::stable_sort(__policy, std::move(__g_first), std::move(__g_last), std::move(__g_comp));
+      },
+      std::move(__first),
+      std::move(__last),
+      std::move(__comp));
+}
+
+template <class _ExecutionPolicy,
+          class _RandomAccessIterator,
+          class _RawPolicy                                    = __remove_cvref_t<_ExecutionPolicy>,
+          enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
+_LIBCPP_HIDE_FROM_ABI void
+sort(_ExecutionPolicy&& __policy, _RandomAccessIterator __first, _RandomAccessIterator __last) {
+  std::sort(std::forward<_ExecutionPolicy>(__policy), std::move(__first), std::move(__last), less{});
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_PSTL) && _LIBCPP_STD_VER >= 17
+
+#endif // _LIBCPP___ALGORITHM_PSTL_SORT_H

libcxx/include/algorithm

 #include <__algorithm/pstl_count.h>
 #include <__algorithm/pstl_fill.h>
 #include <__algorithm/pstl_find.h>
 #include <__algorithm/pstl_for_each.h>
 #include <__algorithm/pstl_generate.h>
 #include <__algorithm/pstl_is_partitioned.h>
 #include <__algorithm/pstl_merge.h>
 #include <__algorithm/pstl_replace.h>
+#include <__algorithm/pstl_sort.h>
 #include <__algorithm/pstl_stable_sort.h>
 #include <__algorithm/pstl_transform.h>
 #include <__algorithm/push_heap.h>
 #include <__algorithm/ranges_adjacent_find.h>
 #include <__algorithm/ranges_all_of.h>
 #include <__algorithm/ranges_any_of.h>
 #include <__algorithm/ranges_binary_search.h>
 #include <__algorithm/ranges_clamp.h>

libcxx/test/libcxx/algorithms/pstl.robust_against_customization_points_not_working.pass.cpp

 template <class, class ForwardIterator>
 typename std::iterator_traits<ForwardIterator>::value_type
 __pstl_reduce(TestBackend, ForwardIterator, ForwardIterator) {
   assert(!pstl_reduce_without_init_called);
   pstl_reduce_without_init_called = true;
   return {};
 }
 
+bool pstl_sort_called = false;
+
+template <class, class RandomAccessIterator, class Comp>
+void __pstl_sort(TestBackend, RandomAccessIterator, RandomAccessIterator, Comp) {
+  assert(!pstl_sort_called);
+  pstl_sort_called = true;
+}
+
+bool pstl_stable_sort_called = false;
+
+template <class, class RandomAccessIterator, class Comp>
+void __pstl_stable_sort(TestBackend, RandomAccessIterator, RandomAccessIterator, Comp) {
+  assert(!pstl_stable_sort_called);
+  pstl_stable_sort_called = true;
+}
+
 bool pstl_unary_transform_reduce_called = false;
 
 template <class, class ForwardIterator, class T, class UnaryOperation, class BinaryOperation>
 T __pstl_transform_reduce(TestBackend, ForwardIterator, ForwardIterator, T, UnaryOperation, BinaryOperation) {
   assert(!pstl_unary_transform_reduce_called);
   pstl_unary_transform_reduce_called = true;
   return {};
 }
int main(int, char**) {
   (void)std::transform(TestPolicy{}, std::begin(a), std::end(a), std::begin(a), pred);
   assert(std::pstl_unary_transform_called);
   (void)std::transform(TestPolicy{}, std::begin(a), std::end(a), std::begin(a), std::begin(a), pred);
   assert(std::pstl_unary_transform_called);
   (void)std::reduce(TestPolicy{}, std::begin(a), std::end(a), 0, pred);
   assert(std::pstl_reduce_with_init_called);
   (void)std::reduce(TestPolicy{}, std::begin(a), std::end(a));
   assert(std::pstl_reduce_without_init_called);
+  (void)std::sort(TestPolicy{}, std::begin(a), std::end(a));
+  assert(std::pstl_sort_called);
+  (void)std::stable_sort(TestPolicy{}, std::begin(a), std::end(a));
+  assert(std::pstl_stable_sort_called);
   (void)std::transform_reduce(TestPolicy{}, std::begin(a), std::end(a), 0, pred, pred);
   assert(std::pstl_unary_transform_reduce_called);
   (void)std::transform_reduce(TestPolicy{}, std::begin(a), std::end(a), std::begin(a), 0, pred, pred);
   assert(std::pstl_binary_transform_reduce_called);
 
   return 0;
 }

libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/pstl.sort.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+
+// UNSUPPORTED: libcpp-has-no-incomplete-pstl
+
+// template<class ExecutionPolicy, class RandomAccessIterator>
+//   void sort(ExecutionPolicy&& exec,
+//             RandomAccessIterator first, RandomAccessIterator last);
+//
+// template<class ExecutionPolicy, class RandomAccessIterator, class Compare>
+//   void sort(ExecutionPolicy&& exec,
+//             RandomAccessIterator first, RandomAccessIterator last,
+//             Compare comp);
+
+#include <algorithm>
+#include <cassert>
+#include <numeric>
+#include <random>
+#include <vector>
+
+#include "test_execution_policies.h"
+#include "test_iterators.h"
+#include "test_macros.h"
+
+template <class Iter>
+struct Test {
+  template <class ExecutionPolicy>
+  void operator()(ExecutionPolicy&& policy) {
+    { // simple test
+      int in[]       = {1, 2, 3, 2, 6, 4};
+      int expected[] = {1, 2, 2, 3, 4, 6};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // empty range works
+      int in[]       = {1, 2, 3, 2, 6, 4};
+      int expected[] = {1, 2, 3, 2, 6, 4};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::begin(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // single element range works
+      int in[]       = {1};
+      int expected[] = {1};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::begin(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // two element range works
+      int in[]       = {2, 1};
+      int expected[] = {1, 2};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // three element range works
+      int in[]       = {2, 1, 4};
+      int expected[] = {1, 2, 4};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // longer range works
+      int in[]       = {2, 1, 4, 4, 7, 2, 4, 1, 6};
+      int expected[] = {1, 1, 2, 2, 4, 4, 4, 6, 7};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // already sorted
+      int in[]       = {1, 1, 2, 2, 4, 4, 4, 6, 7};
+      int expected[] = {1, 1, 2, 2, 4, 4, 4, 6, 7};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // reversed
+      int in[]       = {7, 6, 4, 4, 4, 2, 2, 1, 1};
+      int expected[] = {1, 1, 2, 2, 4, 4, 4, 6, 7};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // repeating pattern
+      int in[]       = {1, 2, 3, 1, 2, 3, 1, 2, 3};
+      int expected[] = {1, 1, 1, 2, 2, 2, 3, 3, 3};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)));
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+    { // a custom comparator is used
+      int in[]       = {1, 2, 3, 2, 6, 4};
+      int expected[] = {6, 4, 3, 2, 2, 1};
+      std::sort(policy, Iter(std::begin(in)), Iter(std::end(in)), std::greater{});
+      assert(std::equal(std::begin(in), std::end(in), std::begin(expected)));
+    }
+#ifndef TEST_HAS_NO_RANDOM_DEVICE
+    { // large range
+      std::vector<int> vec(300);
+      std::iota(std::begin(vec), std::end(vec), 0);
+      auto expected = vec;
+      std::shuffle(std::begin(vec), std::end(vec), std::mt19937_64(std::random_device{}()));
+      std::sort(Iter(std::data(vec)), Iter(std::data(vec) + std::size(vec)));
+      assert(vec == expected);
+    }
+#endif
+  }
+};
+
+int main(int, char**) {
+  types::for_each(types::random_access_iterator_list<int*>{}, TestIteratorWithPolicies<Test>{});
+
+  return 0;
+}

libcxx/test/support/test_macros.h

 #if defined(__GNUC__)
 #define TEST_THROW(...) __builtin_abort()
 #else
 #include <stdlib.h>
 #define TEST_THROW(...) ::abort()
 #endif
 #endif
 
 #if defined(__GNUC__) || defined(__clang__)
 template <class Tp>
 inline
 void DoNotOptimize(Tp const& value) {
     asm volatile("" : : "r,m"(value) : "memory");
 }
 
 template <class Tp>
 inline void DoNotOptimize(Tp& value) {

 #if defined(_LIBCPP_HAS_NO_FGETPOS_FSETPOS)
 #  define TEST_HAS_NO_FGETPOS_FSETPOS
 #endif
 
 #if defined(_LIBCPP_HAS_NO_C8RTOMB_MBRTOC8)
 #  define TEST_HAS_NO_C8RTOMB_MBRTOC8
 #endif
 
+#if defined(_LIBCPP_HAS_NO_RANDOM_DEVICE)
+#  define TEST_HAS_NO_RANDOM_DEVICE
+#endif
+
 #if defined(TEST_COMPILER_CLANG)
 #  define TEST_DIAGNOSTIC_PUSH _Pragma("clang diagnostic push")
 #  define TEST_DIAGNOSTIC_POP _Pragma("clang diagnostic pop")
 #  define TEST_CLANG_DIAGNOSTIC_IGNORED(str) _Pragma(TEST_STRINGIZE(clang diagnostic ignored str))
 #  define TEST_GCC_DIAGNOSTIC_IGNORED(str)
 #  define TEST_MSVC_DIAGNOSTIC_IGNORED(num)
 #elif defined(TEST_COMPILER_GCC)
 #  define TEST_DIAGNOSTIC_PUSH _Pragma("GCC diagnostic push")