[libc][stdlib] Implement heap sort. #98582

lntue · 2024-07-12T02:41:50Z

No description provided.

llvmbot · 2024-07-12T02:42:20Z

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/98582.diff

9 Files Affected:

(modified) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+12-1)
(modified) libc/config/baremetal/config.json (+5)
(modified) libc/config/config.json (+6)
(modified) libc/docs/configure.rst (+2)
(modified) libc/src/stdlib/CMakeLists.txt (+2)
(added) libc/src/stdlib/heap_sort.h (+54)
(modified) libc/src/stdlib/qsort.cpp (+10-2)
(modified) libc/src/stdlib/qsort_r.cpp (+8-2)
(modified) libc/src/stdlib/qsort_util.h (+15-4)

diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index c5e7dfe8abd0f..097d9cb2fd1b9 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -34,10 +34,21 @@ function(_get_compile_options_from_flags output_var)
   set(${output_var} ${compile_options} PARENT_SCOPE)
 endfunction(_get_compile_options_from_flags)
 
+function(_get_compile_options_from_config output_var)
+  set(config_options "")
+
+  if(LIBC_CONF_QSORT_IMPL)
+    list(APPEND config_options "-DLIBC_QSORT_IMPL=${LIBC_CONF_QSORT_IMPL}")
+  endif()
+
+  set(${output_var} ${config_options} PARENT_SCOPE)
+endfunction(_get_compile_options_from_config)
+
 function(_get_common_compile_options output_var flags)
   _get_compile_options_from_flags(compile_flags ${flags})
+  _get_compile_options_from_config(config_flags)
 
-  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags})
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags} ${config_flags})
 
   if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
     list(APPEND compile_options "-fpie")
diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index dda4c42425755..f2bc6e6517f7c 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -17,5 +17,10 @@
     "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
       "value": 102400
     }
+  },
+  "qsort": {
+    "LIBC_CONF_QSORT_IMPL": {
+      "value": "LIBC_QSORT_HEAP_SORT"
+    }
   }
 }
diff --git a/libc/config/config.json b/libc/config/config.json
index e8feab20175f4..e6a65d6689ec7 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -76,5 +76,11 @@
       "value": 0,
       "doc": "Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST."
     }
+  },
+  "qsort": {
+    "LIBC_CONF_QSORT_IMPL": {
+      "value": "LIBC_QSORT_QUICK_SORT",
+      "doc": "Configures sorting algorithm for qsort and qsort_r. Values accepted are LIBC_QSORT_QUICK_SORT, LIBC_QSORT_HEAP_SORT."
+    }
   }
 }
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 9c641ef94570f..777330d815756 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -42,6 +42,8 @@ to learn about the defaults for your platform and target.
     - ``LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a mutex is in contention (default to 100).
     - ``LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a rwlock is in contention (default to 100).
     - ``LIBC_CONF_TIMEOUT_ENSURE_MONOTONICITY``: Automatically adjust timeout to CLOCK_MONOTONIC (default to true). POSIX API may require CLOCK_REALTIME, which can be unstable and leading to unexpected behavior. This option will convert the real-time timestamp to monotonic timestamp relative to the time of call.
+* **"qsort" options**
+    - ``LIBC_CONF_QSORT_IMPL``: Configures sorting algorithm for qsort and qsort_r. Values accepted are LIBC_QSORT_QUICK_SORT, LIBC_QSORT_HEAP_SORT.
 * **"scanf" options**
     - ``LIBC_CONF_SCANF_DISABLE_FLOAT``: Disable parsing floating point values in scanf and friends.
     - ``LIBC_CONF_SCANF_DISABLE_INDEX_MODE``: Disable index mode in the scanf format string.
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 7b7e55db391fa..e19d97b1025d9 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -259,8 +259,10 @@ add_header_library(
   qsort_util
   HDRS
     qsort_util.h
+    heap_sort.h
   DEPENDS
     libc.include.stdlib
+    libc.src.__support.CPP.cstddef
 )
 
 add_entrypoint_object(
diff --git a/libc/src/stdlib/heap_sort.h b/libc/src/stdlib/heap_sort.h
new file mode 100644
index 0000000000000..63aba88fb933f
--- /dev/null
+++ b/libc/src/stdlib/heap_sort.h
@@ -0,0 +1,54 @@
+//===-- Implementation of heap sort -----------------------------*- 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_HEAP_SORT_H
+#define LLVM_LIBC_SRC_STDLIB_HEAP_SORT_H
+
+#include "qsort_util.h"
+#include "src/__support/CPP/cstddef.h"
+
+namespace LIBC_NAMESPACE_DECL {
+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();
+  size_t start = end / 2;
+
+  auto left_child = [](size_t i) -> size_t { return 2 * i + 1; };
+
+  while (end > 1) {
+    if (start > 0) {
+      --start;
+    } else {
+      --end;
+      array.swap(0, end);
+    }
+
+    size_t root = start;
+    while (left_child(root) < end) {
+      size_t child = left_child(root);
+      if (child + 1 < end &&
+          array.elem_compare(child, array.get(child + 1)) < 0)
+        ++child;
+
+      if (array.elem_compare(root, array.get(child)) >= 0)
+        break;
+
+      array.swap(root, child);
+      root = child;
+    }
+  }
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDLIB_HEAP_SORT_H
diff --git a/libc/src/stdlib/qsort.cpp b/libc/src/stdlib/qsort.cpp
index 048e63ab214ed..fb6bfa006eb1f 100644
--- a/libc/src/stdlib/qsort.cpp
+++ b/libc/src/stdlib/qsort.cpp
@@ -9,6 +9,7 @@
 #include "src/stdlib/qsort.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
+#include "src/stdlib/heap_sort.h"
 #include "src/stdlib/qsort_util.h"
 
 #include <stdint.h>
@@ -21,8 +22,15 @@ LLVM_LIBC_FUNCTION(void, qsort,
   if (array == nullptr || array_size == 0 || elem_size == 0)
     return;
   internal::Comparator c(compare);
-  internal::quicksort(internal::Array(reinterpret_cast<uint8_t *>(array),
-                                      array_size, elem_size, c));
+
+  auto arr = internal::Array(reinterpret_cast<uint8_t *>(array), array_size,
+                             elem_size, c);
+
+#if LIBC_QSORT_IMPL == LIBC_QSORT_QUICK_SORT
+  internal::quick_sort(arr);
+#elif LIBC_QSORT_IMPL == LIBC_QSORT_HEAP_SORT
+  internal::heap_sort(arr);
+#endif
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/qsort_r.cpp b/libc/src/stdlib/qsort_r.cpp
index efbe5ad484b0e..3c9bb5aa21f43 100644
--- a/libc/src/stdlib/qsort_r.cpp
+++ b/libc/src/stdlib/qsort_r.cpp
@@ -22,8 +22,14 @@ LLVM_LIBC_FUNCTION(void, qsort_r,
   if (array == nullptr || array_size == 0 || elem_size == 0)
     return;
   internal::Comparator c(compare, arg);
-  internal::quicksort(internal::Array(reinterpret_cast<uint8_t *>(array),
-                                      array_size, elem_size, c));
+  auto arr = internal::Array(reinterpret_cast<uint8_t *>(array), array_size,
+                             elem_size, c);
+
+#if LIBC_QSORT_IMPL == LIBC_QSORT_QUICK_SORT
+  internal::quick_sort(arr);
+#elif LIBC_QSORT_IMPL == LIBC_QSORT_HEAP_SORT
+  internal::heap_sort(arr);
+#endif
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/qsort_util.h b/libc/src/stdlib/qsort_util.h
index 3a9cc4b8669f8..87945c26b3c93 100644
--- a/libc/src/stdlib/qsort_util.h
+++ b/libc/src/stdlib/qsort_util.h
@@ -12,7 +12,18 @@
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/config.h"
 #include <stdint.h>
-#include <stdlib.h>
+
+#define LIBC_QSORT_QUICK_SORT 1
+#define LIBC_QSORT_HEAP_SORT 2
+
+#ifndef LIBC_QSORT_IMPL
+#define LIBC_QSORT_IMPL LIBC_QSORT_QUICK_SORT
+#endif // LIBC_QSORT_IMPL
+
+#if (LIBC_QSORT_IMPL != LIBC_QSORT_QUICK_SORT &&                               \
+     LIBC_QSORT_IMPL != LIBC_QSORT_HEAP_SORT)
+#error "LIBC_QSORT_IMPL is not recognized."
+#endif
 
 namespace LIBC_NAMESPACE_DECL {
 namespace internal {
@@ -136,7 +147,7 @@ static size_t partition(const Array &array) {
   }
 }
 
-LIBC_INLINE void quicksort(const Array &array) {
+LIBC_INLINE void quick_sort(const Array &array) {
   const size_t array_size = array.size();
   if (array_size <= 1)
     return;
@@ -145,8 +156,8 @@ LIBC_INLINE void quicksort(const Array &array) {
     // The partition operation sorts the two element array.
     return;
   }
-  quicksort(array.make_array(0, split_index));
-  quicksort(array.make_array(split_index, array.size() - split_index));
+  quick_sort(array.make_array(0, split_index));
+  quick_sort(array.make_array(split_index, array.size() - split_index));
 }
 
 } // namespace internal

libc/src/stdlib/qsort_r.cpp

PiJoules · 2024-07-12T18:51:31Z

Is it possible to to update qsort_test.cpp such that it can run both sorting algos over the same test cases?

libc/src/stdlib/heap_sort.h

…sort and heap_sort.

lntue · 2024-07-13T01:19:16Z

Is it possible to to update qsort_test.cpp such that it can run both sorting algos over the same test cases?

I added the unit tests for internal::quick_sort and internal::heap_sort directly.

mysterymath

LGTM for me, but see if any of the other reviewers have additional comments.

llvm-ci · 2024-07-16T12:52:36Z

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building libc,utils at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/1937

Here is the relevant piece of the build log for the reference:

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49779.cpp (776 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/test_libc.cpp (777 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/wtime.c (778 of 786)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp (779 of 786)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_target_teams_reduction_min.cpp (780 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/std_complex_arithmetic.cpp (781 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/bug49021.cpp (782 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/complex_reduction.cpp (783 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (784 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49021.cpp (785 of 786)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1228.844356

[libc][stdlib] Implement heap sort.

b19bcbb

lntue requested review from petrhosek, PiJoules and michaelrj-google July 12, 2024 02:41

llvmbot added the libc label Jul 12, 2024

PiJoules requested a review from mysterymath July 12, 2024 04:19

michaelrj-google reviewed Jul 12, 2024

View reviewed changes

libc/src/stdlib/qsort_r.cpp Outdated Show resolved Hide resolved

libc/src/stdlib/qsort_r.cpp Show resolved Hide resolved

mysterymath reviewed Jul 12, 2024

View reviewed changes

Address comments: Refactor qsort_util.h and add unit tests for quick_…

38facab

…sort and heap_sort.

lntue requested review from rupprecht and keith as code owners July 13, 2024 01:17

llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Jul 13, 2024

lntue requested review from mysterymath and michaelrj-google July 13, 2024 01:30

mysterymath approved these changes Jul 15, 2024

View reviewed changes

petrhosek approved these changes Jul 16, 2024

View reviewed changes

lntue merged commit a6d2da8 into llvm:main Jul 16, 2024
8 checks passed

lntue deleted the sort branch July 17, 2024 02:22

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024

[libc][stdlib] Implement heap sort. (#98582)

dc07b8d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc][stdlib] Implement heap sort. #98582

[libc][stdlib] Implement heap sort. #98582

Uh oh!

lntue commented Jul 12, 2024

Uh oh!

llvmbot commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

PiJoules commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lntue commented Jul 13, 2024

Uh oh!

mysterymath left a comment

Uh oh!

Uh oh!

llvm-ci commented Jul 16, 2024

Uh oh!

Uh oh!

[libc][stdlib] Implement heap sort. #98582

[libc][stdlib] Implement heap sort. #98582

Uh oh!

Conversation

lntue commented Jul 12, 2024

Uh oh!

llvmbot commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

PiJoules commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lntue commented Jul 13, 2024

Uh oh!

mysterymath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jul 16, 2024

Uh oh!

Uh oh!