[libcxx-commits] [PATCH] D124122: [libc++] Optimize std::rotate

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Apr 26 13:38:58 PDT 2022

ldionne added a comment.

I'd be fine with this since it addresses the underwhelming performance for simple types like `int`, which is super important. However, I would prefer if we instead went for a better algorithm directly, like the swap/grail rotate algorithm mentioned in https://github.com/llvm/llvm-project/issues/54949#issue-1206295098.

Comment at: libcxx/include/__algorithm/rotate.h:174
     typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;
-    if (is_trivially_move_assignable<value_type>::value)
+    if _LIBCPP_CONSTEXPR_AFTER_CXX14 (is_trivially_move_assignable<value_type>::value && sizeof(value_type) > 32)
I don't think `constexpr` adds a lot of value here since the compiler will definitely fold this anyway, and it's kind of weird to have `_LIBCPP_CONSTEXPR_AFTER_CXX14` in that location.

Comment at: libcxx/include/__algorithm/rotate.h:176
         if (_VSTD::next(__first) == __middle)
             return _VSTD::__rotate_left(__first, __last);
I would also do something like

const bool __is_expensive_to_move = sizeof(value_type) > 32;
if (is_trivially_foo<...> && __is_expensive_to_move) { ... }

That way, the code is somewhat self-documenting.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list