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

[Marco Gelmi via Phabricator via libcxx-commits]

marcogelmi marked an inline comment as done.
marcogelmi added inline comments.


================
Comment at: libcxx/include/__algorithm/sort.h:128
+template <typename _Compare, typename _RandomAccessIterator>
+inline _LIBCPP_HIDDEN void __cond_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _Compare __c) {
+  using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
----------------
ldionne wrote:
> marcogelmi wrote:
> > ldionne wrote:
> > > Also, does the `inline` make a difference in your benchmarks? If not, I'd drop it, because they are semantically equivalent (templates have inline semantics by default).
> > > 
> > > This comment applies below in several places too.
> > Removing the inline seems to negatively affect the performance for some combinations of tests and architectures. Would it be ok to keep the inline for extra safety?
> Can you share those numbers? IMO that's a LLVM bug, since we like to say that `inline` doesn't have any impact on the codegen. But yeah, at the end of the day, if it makes a positive difference we should keep it.
sure, here they are: https://bit.ly/34Q6z3q


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118029