Skip to content

Commit f4b20ba

Browse files
VoultapherVoultapher
committed
Apply review comments
1 parent 850ca5a commit f4b20ba

File tree

10 files changed

+45
-43
lines changed

10 files changed

+45
-43
lines changed

‎libc/src/stdlib/heap_sort.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace internal {
1919
// Follow the implementation in https://en.wikipedia.org/wiki/Heapsort.
2020

2121
template <typename A, typename F>
22-
void heap_sort(const A &array, const F &is_less) {
22+
LIBC_INLINE void heap_sort(const A &array, const F &is_less) {
2323
size_t end = array.len();
2424
size_t start = end / 2;
2525

‎libc/src/stdlib/qsort.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ namespace LIBC_NAMESPACE_DECL {
1818
LLVM_LIBC_FUNCTION(void, qsort,
1919
(void *array, size_t array_size, size_t elem_size,
2020
int (*compare)(const void *, const void *))) {
21-
internal::unstable_sort(array, array_size, elem_size,
22-
[compare](const void *a, const void *b) -> bool {
23-
return compare(a, b) < 0;
24-
});
21+
22+
const auto is_less = [compare](const void *a, const void *b) -> bool {
23+
return compare(a, b) < 0;
24+
};
25+
26+
internal::unstable_sort(array, array_size, elem_size, is_less);
2527
}
2628

2729
} // namespace LIBC_NAMESPACE_DECL

‎libc/src/stdlib/qsort_data.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ class ArrayGenericSize {
2727
}
2828

2929
public:
30-
ArrayGenericSize(void *a, size_t s, size_t e)
30+
LIBC_INLINE ArrayGenericSize(void *a, size_t s, size_t e)
3131
: array_base(reinterpret_cast<cpp::byte *>(a)), array_len(s),
3232
elem_size(e) {}
3333

3434
static constexpr bool has_fixed_size() { return false; }
3535

3636
LIBC_INLINE void *get(size_t i) const { return get_internal(i); }
3737

38-
void swap(size_t i, size_t j) const {
38+
LIBC_INLINE void swap(size_t i, size_t j) const {
3939
// It's possible to use 8 byte blocks with `uint64_t`, but that
4040
// generates more machine code as the remainder loop gets
4141
// unrolled, plus 4 byte operations are more likely to be
@@ -97,7 +97,7 @@ template <size_t ELEM_SIZE> class ArrayFixedSize {
9797
}
9898

9999
public:
100-
ArrayFixedSize(void *a, size_t s)
100+
LIBC_INLINE ArrayFixedSize(void *a, size_t s)
101101
: array_base(reinterpret_cast<cpp::byte *>(a)), array_len(s) {}
102102

103103
// Beware this function is used a heuristic for cheap to swap types, so

‎libc/src/stdlib/qsort_pivot.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,7 @@ size_t median3(const A &array, size_t a, size_t b, size_t c, const F &is_less) {
7373
// If x=y=1 then a < b, c. In this case we want to return min(b, c).
7474
// By toggling the outcome of b < c using XOR x we get this behavior.
7575
const bool z = is_less(b_ptr, c_ptr);
76-
if (z ^ x)
77-
return c;
78-
else
79-
return b;
76+
return z ^ x ? c : b;
8077
} else {
8178
// Either c <= a < b or b <= a < c, thus a is our median.
8279
return a;

‎libc/src/stdlib/qsort_r.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ LLVM_LIBC_FUNCTION(void, qsort_r,
2020
int (*compare)(const void *, const void *, void *),
2121
void *arg)) {
2222

23-
internal::unstable_sort(array, array_size, elem_size,
24-
[compare, arg](const void *a, const void *b) -> bool {
25-
return compare(a, b, arg) < 0;
26-
});
23+
const auto is_less = [compare, arg](const void *a, const void *b) -> bool {
24+
return compare(a, b, arg) < 0;
25+
};
26+
27+
internal::unstable_sort(array, array_size, elem_size, is_less);
2728
}
2829

2930
} // namespace LIBC_NAMESPACE_DECL

‎libc/src/stdlib/qsort_util.h

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ namespace LIBC_NAMESPACE_DECL {
3232
namespace internal {
3333

3434
template <bool USE_QUICKSORT, typename F>
35-
void unstable_sort_impl(void *array, size_t array_len, size_t elem_size,
36-
const F &is_less) {
35+
LIBC_INLINE void unstable_sort_impl(void *array, size_t array_len,
36+
size_t elem_size, const F &is_less) {
3737
if (array == nullptr || array_len == 0 || elem_size == 0)
3838
return;
3939

@@ -68,13 +68,10 @@ void unstable_sort_impl(void *array, size_t array_len, size_t elem_size,
6868
}
6969

7070
template <typename F>
71-
void unstable_sort(void *array, size_t array_len, size_t elem_size,
72-
const F &is_less) {
73-
#if LIBC_QSORT_IMPL == LIBC_QSORT_QUICK_SORT
74-
unstable_sort_impl<true, F>(array, array_len, elem_size, is_less);
75-
#elif LIBC_QSORT_IMPL == LIBC_QSORT_HEAP_SORT
76-
unstable_sort_impl<false, F>(array, array_len, elem_size, is_less);
77-
#endif
71+
LIBC_INLINE void unstable_sort(void *array, size_t array_len, size_t elem_size,
72+
const F &is_less) {
73+
#define USE_QUICK_SORT ((LIBC_QSORT_IMPL) == (LIBC_QSORT_QUICK_SORT))
74+
unstable_sort_impl<USE_QUICK_SORT, F>(array, array_len, elem_size, is_less);
7875
}
7976

8077
} // namespace internal

‎libc/src/stdlib/quick_sort.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ namespace internal {
2424
// https://github.com/Voultapher/sort-research-rs/blob/main/writeup/lomcyc_partition/text.md.
2525
// Simplified to avoid having to stack allocate.
2626
template <typename A, typename F>
27-
size_t partition_lomuto_branchless(const A &array, const void *pivot,
28-
const F &is_less) {
27+
LIBC_INLINE size_t partition_lomuto_branchless(const A &array,
28+
const void *pivot,
29+
const F &is_less) {
2930
const size_t array_len = array.len();
3031

3132
size_t left = 0;
@@ -49,8 +50,8 @@ size_t partition_lomuto_branchless(const A &array, const void *pivot,
4950
// cyclic permutation is to have more efficient swapping, but we don't
5051
// know the element size so this isn't applicable here either.
5152
template <typename A, typename F>
52-
size_t partition_hoare_branchy(const A &array, const void *pivot,
53-
const F &is_less) {
53+
LIBC_INLINE size_t partition_hoare_branchy(const A &array, const void *pivot,
54+
const F &is_less) {
5455
const size_t array_len = array.len();
5556

5657
size_t left = 0;
@@ -78,7 +79,8 @@ size_t partition_hoare_branchy(const A &array, const void *pivot,
7879
}
7980

8081
template <typename A, typename F>
81-
size_t partition(const A &array, size_t pivot_index, const F &is_less) {
82+
LIBC_INLINE size_t partition(const A &array, size_t pivot_index,
83+
const F &is_less) {
8284
// Place the pivot at the beginning of the array.
8385
if (pivot_index != 0) {
8486
array.swap(0, pivot_index);
@@ -104,8 +106,8 @@ size_t partition(const A &array, size_t pivot_index, const F &is_less) {
104106
}
105107

106108
template <typename A, typename F>
107-
void quick_sort_impl(A &array, const void *ancestor_pivot, size_t limit,
108-
const F &is_less) {
109+
LIBC_INLINE void quick_sort_impl(A &array, const void *ancestor_pivot,
110+
size_t limit, const F &is_less) {
109111
while (true) {
110112
const size_t array_len = array.len();
111113
if (array_len <= 1)
@@ -167,7 +169,8 @@ void quick_sort_impl(A &array, const void *ancestor_pivot, size_t limit,
167169

168170
constexpr size_t ilog2(size_t n) { return cpp::bit_width(n) - 1; }
169171

170-
template <typename A, typename F> void quick_sort(A &array, const F &is_less) {
172+
template <typename A, typename F>
173+
LIBC_INLINE void quick_sort(A &array, const F &is_less) {
171174
const void *ancestor_pivot = nullptr;
172175
// Limit the number of imbalanced partitions to `2 * floor(log2(len))`.
173176
// The binary OR by one is used to eliminate the zero-check in the logarithm.

‎libc/test/src/stdlib/SortingTest.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
#include "src/stdlib/qsort.h"
1111
#include "test/UnitTest/Test.h"
1212

13-
#include <iostream>
14-
1513
class SortingTest : public LIBC_NAMESPACE::testing::Test {
1614

1715
using SortingRoutine = void (*)(void *array, size_t array_len,

‎libc/test/src/stdlib/heap_sort_test.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ void heap_sort(void *array, size_t array_size, size_t elem_size,
1414

1515
constexpr bool USE_QUICKSORT = false;
1616

17+
const auto is_less = [compare](const void *a,
18+
const void *b) noexcept -> bool {
19+
return compare(a, b) < 0;
20+
};
21+
1722
LIBC_NAMESPACE::internal::unstable_sort_impl<USE_QUICKSORT>(
18-
array, array_size, elem_size,
19-
[compare](const void *a, const void *b) noexcept -> bool {
20-
return compare(a, b) < 0;
21-
});
23+
array, array_size, elem_size, is_less);
2224
}
2325

2426
LIST_SORTING_TESTS(HeapSort, heap_sort);

‎libc/test/src/stdlib/quick_sort_test.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ void quick_sort(void *array, size_t array_size, size_t elem_size,
1313
int (*compare)(const void *, const void *)) {
1414
constexpr bool USE_QUICKSORT = true;
1515

16+
const auto is_less = [compare](const void *a,
17+
const void *b) noexcept -> bool {
18+
return compare(a, b) < 0;
19+
};
20+
1621
LIBC_NAMESPACE::internal::unstable_sort_impl<USE_QUICKSORT>(
17-
array, array_size, elem_size,
18-
[compare](const void *a, const void *b) noexcept -> bool {
19-
return compare(a, b) < 0;
20-
});
22+
array, array_size, elem_size, is_less);
2123
}
2224

2325
LIST_SORTING_TESTS(Qsort, quick_sort);

0 commit comments

Comments
 (0)