[libc-commits] [libc] [llvm] Improve qsort (PR #120450)
via libc-commits
libc-commits at lists.llvm.org
Wed Dec 18 08:42:32 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Lukas Bergdoll (Voultapher)
<details>
<summary>Changes</summary>
This PR improves the libc `qsort` and `qsort_r` implementation in a variety of ways:
- Vastly improved performance, 2+x improvement in many scenarios, see below.
- Improved usage safety, the current implementation allows for trivial
out-of-bounds read and write UB if the comparison function does not implement
a strict weak ordering. I've written about this in more depth
[here](https://github.com/Voultapher/sort-research-rs/blob/main/writeup/sort_safety/text.md#ord-safety).
For reference glibc qsort considers this a [safety critical
bug](https://www.openwall.com/lists/oss-security/2024/01/30/7) and does bounds
checking. The current implementation contains code like this `while
((compare_i = array.elem_compare(i, pivot)) < 0) ++i;` which goes
out-of-bounds if the comparison function does not implement a strict weak
ordering. The new implementation either does bounds checking or loop bounds
independent of the comparison result.
- Better worst case algorithmic complexity. The current implementation is a classical quicksort implementation with a worst case expected time to sort the data of O(N^2). The new implementation improves that to O(N x log(N)) by limiting the recursion depth and switching to heapsort if a limit is hit, this idea comes from introsort. In addition the new implementation achieves O(N x log(K)) where K is the number of distinct elements by using equal element filtering as pioneered by [pdqsort](https://github.com/orlp/pdqsort).
There is one downside, binary-size. The current implementation is very basic and uses dynamic dispatch to re-use the same implementation for `qsort` and `qsort_r`. In practice with default `-O3` settings on x86-64 the binary-size `qsort` and `qsort_r` contribute in total to libc goes from 1kB to 18kB, this is a net 17kB increase. It's possible to reduce binary-size further but doing so comes at the price of significant reduction in performance. The new implementation-- idisort (ipnsort derived introsort sort) - makes rather conservative choices, with a higher binary-size budget things like efficient full ascending and descending handling or more size specializations are possible.
For some background, together with @<!-- -->orlp I worked on improving the Rust standard library sort implementations which was merged earlier this year and is part of stable Rust for several month now. The ideas in idisort are largely derived from the learnings gained by developing [ipnsort](https://github.com/Voultapher/sort-research-rs/blob/main/writeup/ipnsort_introduction/text.md). The major performance difference comes from having an opaque comparison function as forced by the `qsort` interface, which limits the kind of optimizations that can be done in code an by the compiler.
### Benchmark setup
```
Linux 6.11
clang version 18.1.8
rustc 1.85.0-nightly (28fc2ba71 2024-11-24)
AMD Ryzen 9 5900X 12-Core Processor (Zen 3 micro-architecture)
CPU boost enabled.
```
### Benchmark results
The used methodology is explained in detail [here](https://github.com/Voultapher/sort-research-rs/blob/main/writeup/ipnsort_introduction/text.md#efficient).
The *current* LLVM libc qsort implementation is referenced as `c_llvm_libc_unstable`, the new implementation is referenced as `c_idisort_unstable`. There are other implementations included for a comparison of the state of the art.
<img src="https://github.com/user-attachments/assets/13592b7c-edd1-4e4a-998a-6e1da9593d69" width=700 />
This graph compares the performance of two implementations against each other, values above zero imply speedups for the new implementation and values below zero imply slowdowns. Each dot represents one input length for a specific pattern combination.
<img src="https://github.com/user-attachments/assets/2d64bd19-bf55-4de8-aab3-6848ff2658da" width=960 />
Expand this for additional benchmark results with other types:
<details>
<img src="https://github.com/user-attachments/assets/b3f0ea45-64ef-4601-a376-f14d6f199595" width=960 />
<img src="https://github.com/user-attachments/assets/a618dca0-2304-4fcd-9923-aee108d34b2c" width=960 />
`random_p5` at an input length of 22k peaks at a 558x performance improvement.
Keep in mind this is an artifical test. But generally large types see quite bad performance in the current implementation.
<img src="https://github.com/user-attachments/assets/c99d5795-c77b-49d7-903c-7ff71b6746d8" width=960 />
<img src="https://github.com/user-attachments/assets/cf3ef6fd-656a-4199-b567-d70e4136f074" width=960 />
</details>
This PR is best reviewed by individual commits, they represent logical increments that are all individually buildable and correct. The commit messages explain the reasoning behind changes in more depth.
This is my first time contributing to the LLVM libc so please point out structural mistakes.
---
Patch is 39.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120450.diff
14 Files Affected:
- (modified) libc/src/stdlib/heap_sort.h (+6-6)
- (modified) libc/src/stdlib/qsort.cpp (+5-8)
- (modified) libc/src/stdlib/qsort_data.h (+128-69)
- (added) libc/src/stdlib/qsort_pivot.h (+92)
- (modified) libc/src/stdlib/qsort_r.cpp (+5-6)
- (modified) libc/src/stdlib/qsort_util.h (+37-2)
- (modified) libc/src/stdlib/quick_sort.h (+144-56)
- (modified) libc/test/src/stdlib/CMakeLists.txt (+1-13)
- (modified) libc/test/src/stdlib/SortingTest.h (+60-103)
- (modified) libc/test/src/stdlib/heap_sort_test.cpp (+15-4)
- (modified) libc/test/src/stdlib/qsort_r_test.cpp (+2-2)
- (modified) libc/test/src/stdlib/qsort_test.cpp (+9-5)
- (removed) libc/test/src/stdlib/quick_sort_test.cpp (-16)
- (modified) utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel (+2-10)
``````````diff
diff --git a/libc/src/stdlib/heap_sort.h b/libc/src/stdlib/heap_sort.h
index ccb9ec5f82149e..de08984497782a 100644
--- a/libc/src/stdlib/heap_sort.h
+++ b/libc/src/stdlib/heap_sort.h
@@ -18,11 +18,12 @@ namespace internal {
// A simple in-place heapsort implementation.
// Follow the implementation in https://en.wikipedia.org/wiki/Heapsort.
-LIBC_INLINE void heap_sort(const Array &array) {
- size_t end = array.size();
+template <typename A, typename F>
+void heap_sort(const A &array, const F &is_less) {
+ size_t end = array.len();
size_t start = end / 2;
- auto left_child = [](size_t i) -> size_t { return 2 * i + 1; };
+ const auto left_child = [](size_t i) -> size_t { return 2 * i + 1; };
while (end > 1) {
if (start > 0) {
@@ -40,12 +41,11 @@ LIBC_INLINE void heap_sort(const Array &array) {
while (left_child(root) < end) {
size_t child = left_child(root);
// If there are two children, set child to the greater.
- if (child + 1 < end &&
- array.elem_compare(child, array.get(child + 1)) < 0)
+ if ((child + 1 < end) && is_less(array.get(child), array.get(child + 1)))
++child;
// If the root is less than the greater child
- if (array.elem_compare(root, array.get(child)) >= 0)
+ if (!is_less(array.get(root), array.get(child)))
break;
// Swap the root with the greater child and continue sifting down.
diff --git a/libc/src/stdlib/qsort.cpp b/libc/src/stdlib/qsort.cpp
index 65a63c239f5c0d..1402f7963c389e 100644
--- a/libc/src/stdlib/qsort.cpp
+++ b/libc/src/stdlib/qsort.cpp
@@ -18,14 +18,11 @@ namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(void, qsort,
(void *array, size_t array_size, size_t elem_size,
int (*compare)(const void *, const void *))) {
- if (array == nullptr || array_size == 0 || elem_size == 0)
- return;
- internal::Comparator c(compare);
-
- auto arr = internal::Array(reinterpret_cast<uint8_t *>(array), array_size,
- elem_size, c);
-
- internal::sort(arr);
+ internal::unstable_sort(
+ array, array_size, elem_size,
+ [compare](const void *a, const void *b) noexcept -> bool {
+ return compare(a, b) < 0;
+ });
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/qsort_data.h b/libc/src/stdlib/qsort_data.h
index c529d55ca46ffd..f7446ec2bb637d 100644
--- a/libc/src/stdlib/qsort_data.h
+++ b/libc/src/stdlib/qsort_data.h
@@ -11,96 +11,155 @@
#include "src/__support/CPP/cstddef.h"
#include "src/__support/macros/config.h"
+#include "src/string/memory_utils/inline_memcpy.h"
+#include "src/string/memory_utils/inline_memmove.h"
#include <stdint.h>
namespace LIBC_NAMESPACE_DECL {
namespace internal {
-using Compare = int(const void *, const void *);
-using CompareWithState = int(const void *, const void *, void *);
-
-enum class CompType { COMPARE, COMPARE_WITH_STATE };
-
-struct Comparator {
- union {
- Compare *comp_func;
- CompareWithState *comp_func_r;
- };
- const CompType comp_type;
-
- void *arg;
-
- Comparator(Compare *func)
- : comp_func(func), comp_type(CompType::COMPARE), arg(nullptr) {}
-
- Comparator(CompareWithState *func, void *arg_val)
- : comp_func_r(func), comp_type(CompType::COMPARE_WITH_STATE),
- arg(arg_val) {}
-
-#if defined(__clang__)
- // Recent upstream changes to -fsanitize=function find more instances of
- // function type mismatches. One case is with the comparator passed to this
- // class. Libraries will tend to pass comparators that take pointers to
- // varying types while this comparator expects to accept const void pointers.
- // Ideally those tools would pass a function that strictly accepts const
- // void*s to avoid UB, or would use qsort_r to pass their own comparator.
- [[clang::no_sanitize("function")]]
-#endif
- int comp_vals(const void *a, const void *b) const {
- if (comp_type == CompType::COMPARE) {
- return comp_func(a, b);
- } else {
- return comp_func_r(a, b, arg);
- }
+// Returns the max amount of bytes deemed reasonable - based on the target
+// properties - for use in local stack arrays.
+constexpr size_t max_stack_array_size() {
+ // Uses target pointer size as heuristic how much memory is available and
+ // unlikely to run into stack overflow and perf problems.
+ constexpr size_t ptr_diff_size = sizeof(ptrdiff_t);
+
+ if constexpr (ptr_diff_size >= 8) {
+ return 4096;
}
-};
-class Array {
- uint8_t *array;
- size_t array_size;
+ if constexpr (ptr_diff_size == 4) {
+ return 512;
+ }
+
+ // 8-bit platforms are just not gonna work well with libc, qsort
+ // won't be the problem.
+ // 16-bit platforms ought to be able to store 64 bytes on the stack.
+ return 64;
+}
+
+class ArrayGenericSize {
+ uint8_t *array_base;
+ size_t array_len;
size_t elem_size;
- Comparator compare;
+
+ uint8_t *get_internal(size_t i) const noexcept {
+ return array_base + (i * elem_size);
+ }
public:
- Array(uint8_t *a, size_t s, size_t e, Comparator c)
- : array(a), array_size(s), elem_size(e), compare(c) {}
-
- uint8_t *get(size_t i) const { return array + i * elem_size; }
-
- void swap(size_t i, size_t j) const {
- uint8_t *elem_i = get(i);
- uint8_t *elem_j = get(j);
- for (size_t b = 0; b < elem_size; ++b) {
- uint8_t temp = elem_i[b];
- elem_i[b] = elem_j[b];
- elem_j[b] = temp;
- }
+ ArrayGenericSize(uint8_t *a, size_t s, size_t e) noexcept
+ : array_base(a), array_len(s), elem_size(e) {}
+
+ static constexpr bool has_fixed_size() { return false; }
+
+ void *get(size_t i) const noexcept {
+ return reinterpret_cast<void *>(get_internal(i));
}
- int elem_compare(size_t i, const uint8_t *other) const {
- // An element must compare equal to itself so we don't need to consult the
- // user provided comparator.
- if (get(i) == other)
- return 0;
- return compare.comp_vals(get(i), other);
+ void swap(size_t i, size_t j) const noexcept {
+ // It's possible to use 8 byte blocks with `uint64_t`, but that
+ // generates more machine code as the remainder loop gets
+ // unrolled, plus 4 byte operations are more likely to be
+ // efficient on a wider variety of hardware. On x86 LLVM tends
+ // to unroll the block loop again into 2 16 byte swaps per
+ // iteration which is another reason that 4 byte blocks yields
+ // good performance even for big types.
+ using block_t = uint32_t;
+ constexpr size_t BLOCK_SIZE = sizeof(block_t);
+
+ uint8_t *elem_i = get_internal(i);
+ uint8_t *elem_j = get_internal(j);
+
+ const size_t elem_size_rem = elem_size % BLOCK_SIZE;
+ const block_t *elem_i_block_end =
+ reinterpret_cast<block_t *>(elem_i + (elem_size - elem_size_rem));
+
+ block_t *elem_i_block = reinterpret_cast<block_t *>(elem_i);
+ block_t *elem_j_block = reinterpret_cast<block_t *>(elem_j);
+
+ while (elem_i_block != elem_i_block_end) {
+ block_t tmp = *elem_i_block;
+ *elem_i_block = *elem_j_block;
+ *elem_j_block = tmp;
+ elem_i_block += 1;
+ elem_j_block += 1;
+ }
+
+ elem_i = reinterpret_cast<uint8_t *>(elem_i_block);
+ elem_j = reinterpret_cast<uint8_t *>(elem_j_block);
+ for (size_t n = 0; n < elem_size_rem; ++n) {
+ uint8_t tmp = elem_i[n];
+ elem_i[n] = elem_j[n];
+ elem_j[n] = tmp;
+ }
}
- size_t size() const { return array_size; }
+ size_t len() const noexcept { return array_len; }
- // Make an Array starting at index |i| and size |s|.
- LIBC_INLINE Array make_array(size_t i, size_t s) const {
- return Array(get(i), s, elem_size, compare);
+ // Make an Array starting at index |i| and length |s|.
+ ArrayGenericSize make_array(size_t i, size_t s) const noexcept {
+ return ArrayGenericSize(get_internal(i), s, elem_size);
}
- // Reset this Array to point at a different interval of the same items.
- LIBC_INLINE void reset_bounds(uint8_t *a, size_t s) {
- array = a;
- array_size = s;
+ // Reset this Array to point at a different interval of the same
+ // items starting at index |i|.
+ void reset_bounds(size_t i, size_t s) noexcept {
+ array_base = get_internal(i);
+ array_len = s;
}
};
-using SortingRoutine = void(const Array &);
+// Having a specialized Array type for sorting that knowns at
+// compile-time what the size of the element is, allows for much more
+// efficient swapping and for cheaper offset calculations.
+template <size_t ELEM_SIZE> class ArrayFixedSize {
+ uint8_t *array_base;
+ size_t array_len;
+
+ uint8_t *get_internal(size_t i) const noexcept {
+ return array_base + (i * ELEM_SIZE);
+ }
+
+public:
+ ArrayFixedSize(uint8_t *a, size_t s) noexcept : array_base(a), array_len(s) {}
+
+ // Beware this function is used a heuristic for cheap to swap types, so
+ // instantiating `ArrayFixedSize` with `ELEM_SIZE > 100` is probably a bad
+ // idea perf wise.
+ static constexpr bool has_fixed_size() { return true; }
+
+ void *get(size_t i) const noexcept {
+ return reinterpret_cast<void *>(get_internal(i));
+ }
+
+ void swap(size_t i, size_t j) const noexcept {
+ alignas(32) uint8_t tmp[ELEM_SIZE];
+
+ uint8_t *elem_i = get_internal(i);
+ uint8_t *elem_j = get_internal(j);
+
+ inline_memcpy(tmp, elem_i, ELEM_SIZE);
+ inline_memmove(elem_i, elem_j, ELEM_SIZE);
+ inline_memcpy(elem_j, tmp, ELEM_SIZE);
+ }
+
+ size_t len() const noexcept { return array_len; }
+
+ // Make an Array starting at index |i| and length |s|.
+ ArrayFixedSize<ELEM_SIZE> make_array(size_t i, size_t s) const noexcept {
+ return ArrayFixedSize<ELEM_SIZE>(get_internal(i), s);
+ }
+
+ // Reset this Array to point at a different interval of the same
+ // items starting at index |i|.
+ void reset_bounds(size_t i, size_t s) noexcept {
+ array_base = get_internal(i);
+ array_len = s;
+ }
+};
} // namespace internal
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/qsort_pivot.h b/libc/src/stdlib/qsort_pivot.h
new file mode 100644
index 00000000000000..68716d01c47a0b
--- /dev/null
+++ b/libc/src/stdlib/qsort_pivot.h
@@ -0,0 +1,92 @@
+//===-- Implementation header for qsort utilities ---------------*- C++ -*-===//
+//
+// 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 LLVM_LIBC_SRC_STDLIB_QSORT_PIVOT_H
+#define LLVM_LIBC_SRC_STDLIB_QSORT_PIVOT_H
+
+#include "src/stdlib/qsort_pivot.h"
+
+#include <stdint.h>
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+
+// Recursively select a pseudomedian if above this threshold.
+constexpr size_t PSEUDO_MEDIAN_REC_THRESHOLD = 64;
+
+// Selects a pivot from `array`. Algorithm taken from glidesort by Orson Peters.
+//
+// This chooses a pivot by sampling an adaptive amount of points, approximating
+// the quality of a median of sqrt(n) elements.
+template <typename A, typename F>
+size_t choose_pivot(const A &array, const F &is_less) {
+ const size_t len = array.len();
+
+ if (len < 8) {
+ return 0;
+ }
+
+ const size_t len_div_8 = len / 8;
+
+ const size_t a = 0; // [0, floor(n/8))
+ const size_t b = len_div_8 * 4; // [4*floor(n/8), 5*floor(n/8))
+ const size_t c = len_div_8 * 7; // [7*floor(n/8), 8*floor(n/8))
+
+ if (len < PSEUDO_MEDIAN_REC_THRESHOLD) {
+ return median3(array, a, b, c, is_less);
+ } else {
+ return median3_rec(array, a, b, c, len_div_8, is_less);
+ }
+}
+
+// Calculates an approximate median of 3 elements from sections a, b, c, or
+// recursively from an approximation of each, if they're large enough. By
+// dividing the size of each section by 8 when recursing we have logarithmic
+// recursion depth and overall sample from f(n) = 3*f(n/8) -> f(n) =
+// O(n^(log(3)/log(8))) ~= O(n^0.528) elements.
+template <typename A, typename F>
+size_t median3_rec(const A &array, size_t a, size_t b, size_t c, size_t n,
+ const F &is_less) {
+ if (n * 8 >= PSEUDO_MEDIAN_REC_THRESHOLD) {
+ const size_t n8 = n / 8;
+ a = median3_rec(array, a, a + (n8 * 4), a + (n8 * 7), n8, is_less);
+ b = median3_rec(array, b, b + (n8 * 4), b + (n8 * 7), n8, is_less);
+ c = median3_rec(array, c, c + (n8 * 4), c + (n8 * 7), n8, is_less);
+ }
+ return median3(array, a, b, c, is_less);
+}
+
+/// Calculates the median of 3 elements.
+template <typename A, typename F>
+size_t median3(const A &array, size_t a, size_t b, size_t c, const F &is_less) {
+ const void *a_ptr = array.get(a);
+ const void *b_ptr = array.get(b);
+ const void *c_ptr = array.get(c);
+
+ const bool x = is_less(a_ptr, b_ptr);
+ const bool y = is_less(a_ptr, c_ptr);
+ if (x == y) {
+ // If x=y=0 then b, c <= a. In this case we want to return max(b, c).
+ // If x=y=1 then a < b, c. In this case we want to return min(b, c).
+ // By toggling the outcome of b < c using XOR x we get this behavior.
+ const bool z = is_less(b_ptr, c_ptr);
+ if (z ^ x) {
+ return c;
+ } else {
+ return b;
+ }
+ } else {
+ // Either c <= a < b or b <= a < c, thus a is our median.
+ return a;
+ }
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDLIB_QSORT_PIVOT_H
diff --git a/libc/src/stdlib/qsort_r.cpp b/libc/src/stdlib/qsort_r.cpp
index bf61a40e847341..1d340560bcbe12 100644
--- a/libc/src/stdlib/qsort_r.cpp
+++ b/libc/src/stdlib/qsort_r.cpp
@@ -19,13 +19,12 @@ LLVM_LIBC_FUNCTION(void, qsort_r,
(void *array, size_t array_size, size_t elem_size,
int (*compare)(const void *, const void *, void *),
void *arg)) {
- if (array == nullptr || array_size == 0 || elem_size == 0)
- return;
- internal::Comparator c(compare, arg);
- auto arr = internal::Array(reinterpret_cast<uint8_t *>(array), array_size,
- elem_size, c);
- internal::sort(arr);
+ internal::unstable_sort(
+ array, array_size, elem_size,
+ [compare, arg](const void *a, const void *b) noexcept -> bool {
+ return compare(a, b, arg) < 0;
+ });
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/qsort_util.h b/libc/src/stdlib/qsort_util.h
index d42adde06d9762..ee3f737926f530 100644
--- a/libc/src/stdlib/qsort_util.h
+++ b/libc/src/stdlib/qsort_util.h
@@ -27,11 +27,46 @@
namespace LIBC_NAMESPACE_DECL {
namespace internal {
+template <typename A, typename F> void sort_inst(A &array, const F &is_less) {
#if LIBC_QSORT_IMPL == LIBC_QSORT_QUICK_SORT
-constexpr auto sort = quick_sort;
+ quick_sort(array, is_less);
#elif LIBC_QSORT_IMPL == LIBC_QSORT_HEAP_SORT
-constexpr auto sort = heap_sort;
+ heap_sort(array, is_less);
#endif
+}
+
+template <typename F>
+void unstable_sort(void *array, size_t array_len, size_t elem_size,
+ const F &is_less) {
+ if (array == nullptr || array_len == 0 || elem_size == 0) {
+ return;
+ }
+
+ uint8_t *array_base = reinterpret_cast<uint8_t *>(array);
+
+ switch (elem_size) {
+ case 4: {
+ auto arr_fixed_size = internal::ArrayFixedSize<4>(array_base, array_len);
+ sort_inst(arr_fixed_size, is_less);
+ return;
+ }
+ case 8: {
+ auto arr_fixed_size = internal::ArrayFixedSize<8>(array_base, array_len);
+ sort_inst(arr_fixed_size, is_less);
+ return;
+ }
+ case 16: {
+ auto arr_fixed_size = internal::ArrayFixedSize<16>(array_base, array_len);
+ sort_inst(arr_fixed_size, is_less);
+ return;
+ }
+ default:
+ auto arr_generic_size =
+ internal::ArrayGenericSize(array_base, array_len, elem_size);
+ sort_inst(arr_generic_size, is_less);
+ return;
+ }
+}
} // namespace internal
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/quick_sort.h b/libc/src/stdlib/quick_sort.h
index 82b90a7d511d99..13cce7ee5d86cd 100644
--- a/libc/src/stdlib/quick_sort.h
+++ b/libc/src/stdlib/quick_sort.h
@@ -9,84 +9,172 @@
#ifndef LLVM_LIBC_SRC_STDLIB_QUICK_SORT_H
#define LLVM_LIBC_SRC_STDLIB_QUICK_SORT_H
-#include "src/__support/macros/attributes.h"
+#include "src/__support/CPP/cstddef.h"
+#include "src/__support/big_int.h"
#include "src/__support/macros/config.h"
-#include "src/stdlib/qsort_data.h"
+#include "src/stdlib/qsort_pivot.h"
#include <stdint.h>
namespace LIBC_NAMESPACE_DECL {
namespace internal {
-// A simple quicksort implementation using the Hoare partition scheme.
-LIBC_INLINE size_t partition(const Array &array) {
- const size_t array_size = array.size();
- size_t pivot_index = array_size / 2;
- uint8_t *pivot = array.get(pivot_index);
- size_t i = 0;
- size_t j = array_size - 1;
+// Branchless Lomuto partition based on the implementation by Lukas
+// Bergdoll and Orson Peters
+// https://github.com/Voultapher/sort-research-rs/blob/main/writeup/lomcyc_partition/text.md.
+// Simplified to avoid having to stack allocate.
+template <typename A, typename F>
+size_t partition_lomuto_branchless(const A &array, const void *pivot,
+ const F &is_less) {
+ const size_t array_len = array.len();
+
+ size_t left = 0;
+ size_t right = 0;
+
+ while (right < array_len) {
+ const bool right_is_lt = is_less(array.get(right), pivot);
+ array.swap(left, right);
+ left += static_cast<size_t>(right_is_lt);
+ right += 1;
+ }
+
+ return left;
+}
+
+// Optimized for large types that are expensive to move. Not optimized
+// for integers. It's possible to use a cyclic permutation here for
+// large types as done in ipnsort but the advantages of this are limited
+// as `is_less` is a small wrapper around a call to a function pointer
+// and won't incur much binary-size overhead. The other reason to use
+// cyclic permutation is to have more efficient swapping, but we don't
+// know the element size so this isn't applicable here either.
+template <typename A, typename F>
+size_t partition_hoare_branchy(const A &array, const void *pivot,
+ const F &is_less) {
+ const size_t array_len = array.len();
+
+ size_t left = 0;
+ size_t right = array_len;
while (true) {
- int compare_i, compare_j;
-
- while ((compare_i = array.elem_compare(i, pivot)) < 0)
- ++i;
- while ((compare_j = array.elem_compare(j, pivot)) > 0)
- --j;
-
- // At some point i will crossover j so we will definitely break out of
- // this while loop.
- if (i >= j)
- return j + 1;
-
- array.swap(i, j);
-
- // The pivot itself might have got swapped so we will update the pivot.
- if (i == pivot_index) {
- pivot = array.get(j);
- pivot_index = j;
- } else if (j == pivot_index) {
- pivot = array.get(i);
- pivot_index = i;
+ while (left < right && is_less(array.get(left), pivot))
+ ++left;
+
+ while (true) {
+ --right;
+ if (left >= right || is_less(array.get(right), pivot)) {
+ break;
+ }
}
- if (compare_i == 0 && compare_j == 0) {
- // If we do not move the pointers, we will end up with an
- // infinite loop as i and j will be stuck without advancing.
- ++i;
- --j;
- }
+ if (left >= right)
+ break;
+
+ array.swap(left, right);
+ ++left;
+ }
+
+ return left;
+}
+
+template <typename A, typename F>
+size_t partition(const A &array, size_t pivot_index, const F &is_less) {
+ // Place the pivot at the beginning of the array.
+ if (pivot_index != 0) {
+ array.swap(0, pivot_index);
}
+
+ const A array_without_pivot = array.make_array(1, array.len() - 1);
+ const void *pivot = array.get(0);
+
+ size_t num_lt;
+ if constexpr (A::has_fixed_size()) {
+ // Branchless Lomuto avoid branch misprediction penalties, but
+ // it also swaps more often which only is faster if the swap a
+ // constant operation.
+ num_lt = partition_lomuto_branchless(array_without_pivot, pivot, is_less);
+ } else {
+ num_lt = partition_hoare_branchy(array_without_pivot, pivot, is_less);
+ }
+
+ // Place the pivot between the two partitions.
+ array.swap(0, num_lt);
+
+ return num_lt;
}
-LIBC_INLINE void quick_sort(Array array) {
+template <typename A, typename F>
+void quick_sort_impl(A &array, const void *ancestor_pivot, size_t limit,
+ ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/120450
More information about the libc-commits
mailing list