[libcxx-commits] [PATCH] D118029: Introduce branchless sorting functions for sort3, sort4 and sort5.

Mon Feb 21 12:34:23 PST 2022

philnik added a comment.

In D118029#3335871 <https://reviews.llvm.org/D118029#3335871>, @Quuxplusone wrote:

> FWIW, I have no strong desire to be obstructionist, and I definitely don't think we should //abandon// this direction, but I have one bad and one good(?) reason I think we should hold this PR in suspended animation for a while longer:
>
> - Bad: I'm still uncomfortable with the invasiveness of the changes to `robust_against_copying_comparators.pass.cpp`.
> - Good(?): Very soon now we're going to have to implement `ranges::sort`, which innovates in a bunch of different directions — we're going to have to deal with sentinels as well as iterators, and we're going to have to replace `__c(*x, *y)` with `invoke(__c, invoke(__p, *x), invoke(__p, *y)`, and so on. I feel like it would be a good idea to get //that// stuff out of the way first, and then revisit algorithmic changes like this PR, rather than vice versa. Unfortunately I have no particular estimate of when `ranges::sort` will get done; @philnik might have a long-range plan to share, but being as we're still down in the foothills of `ranges::min_element` etc., personally I'd predict it'll be a couple months //at least//.
>
> Anyway, @ldionne will be the ultimate arbiter (but IIRC he's on vacation this week).
>
> I'll also keep recommending that you replace `is_arithmetic` with `is_scalar`, until you actually do it! or explain why not. :)

I think `ranges::sort` is one of the last algorithms on the list. Unlike the ones I'm working on currently, the most naive implementation isn't the fastest. So it will be much more work, and even the ones currently under review take a long time, since we don't have many copy/paste-able tests currently. Assuming we will get faster I'd guess the earliest I would start to implement sort is in 4-6 months. I wouldn't be surprised if sort alone will be under review for a few months.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118029