[libcxx-commits] [PATCH] D92190: [libc++] fix std::sort(T**, T**)
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Dec 8 08:31:11 PST 2020
ldionne added a subscriber: howard.hinnant.
ldionne added a comment.
In D92190#2435062 <https://reviews.llvm.org/D92190#2435062>, @Quuxplusone wrote:
> Aw, I wish I had seen this to bikeshed over it — it's relevant to my interests! (Finishing P0879 constexpr algorithms which includes `sort`; the nightmare minefield that is comparing pointers https://quuxplusone.github.io/blog/2019/01/20/std-less-nightmare/)
> - Casting `T**` to `any-kind-of-int*` isn't something you can do constexprly, is it?
No, since it's a `reinterpret_cast` in essence.
> - Does this `sort` overload exist //purely// to avoid instantiating `std::less<T*>`? why is that so important? (`git blame` offers no help; this codepath dates back to Howard's initial import.)
My hunch is that it's a manual attempt to perform outlining, i.e. to avoid instantiating the same `std::sort` code over and over again for different pointer types when they are performing essentially the same task on objects that have the same representation. I could be wrong, I'm just guessing.
> - Could this `sort` overload simply delegate to `_VSTD::sort(first, last, __less<void*>())`, to avoid the constexpr-unfriendly part? (That is, don't type-pun the actual elements; force an implicit conversion to `void*` before each comparison.)
If that results in worse generated code, then we'll have a difficult tradeoff to make. But if not, then I agree we could do that. Or we could drop the outlining attempt altogether. It all depends on whether writing `sort` that way actually provides a benefit with modern compilers or not, I think. We'd have to check.
Pinging @howard.hinnant in case he remembers what was the reason for writing `sort` that way.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits