[PATCH] D93233: [libc++] Replaces std::sort by Bitset sorting algorithm.

Nilay Vaish via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 9 08:34:46 PST 2021


nilayvaish added a comment.

In D93233#3118424 <https://reviews.llvm.org/D93233#3118424>, @ldionne wrote:

> In D93233#3101571 <https://reviews.llvm.org/D93233#3101571>, @nilayvaish wrote:
>
>> Hi ldionne@, I have question for you.  Do the tests we have for libc++ ensure that the new implementation works well across all versions of the C++ standard?  I want to make sure that I am not going to unintentionally break someone's standard-compliant code out there.
>
> They should, however you're welcome to look at them - our tests are sometimes not up to our quality standards, so you should make sure they will catch all issues you can think about.
>
> We do run those tests in C++03, C++11, C++14, C++17, C++20 and C++23 modes, if that was the sole intent of your question.

I think the current tests do not cover some of the ways in which folks use sort.  I did some internal testing and that revealed few compilation issues with one of the previous versions of the diff.  I'll try to reproduce those issues and add those to tests.

> In D93233#3116956 <https://reviews.llvm.org/D93233#3116956>, @nilayvaish wrote:
>
>> ldionne@, I am going to break this change into few parts and provide the benchmark results for each of the part separately.  To begin with, I have posted: https://reviews.llvm.org/D113413 that makes the introsort change to the std::sort implementation.
>
> That's fine by me if you prefer doing it that way, however at the moment I am failing to understand where https://reviews.llvm.org/D113413 fits into the picture. Can you explain what's the plan? Basically introduce introsort in D113413 <https://reviews.llvm.org/D113413> and then replace it by this bitset sort? Sorry if that's a stupid question.

D93233 <https://reviews.llvm.org/D93233> has three different changes in it which I would like to break into parts.  So the plan is to do the following:

- D113413 <https://reviews.llvm.org/D113413>: introduce introsort  (improves worst case from O(n^2) to O(n log n).
- D93233 <https://reviews.llvm.org/D93233> or another commit: introduce bitset sort (improves the core of the quicksort implementation to avoid branch mispredictions)
- D93233 <https://reviews.llvm.org/D93233> or another commit: improve small sized sort (improves small sized sorting so that it is possibly vectorized by the compiler and the number of comparisons in use is optimal).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93233/new/

https://reviews.llvm.org/D93233



More information about the llvm-commits mailing list