[libcxx-commits] [libcxx] [lldb] [libcxx] adds a size-based representation for `vector`'s unstable ABI (PR #155330)

[Louis Dionne via libcxx-commits]

================
@@ -462,19 +468,24 @@ class vector {
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(value_type&& __x) { emplace_back(std::move(__x)); }
 
   template <class... _Args>
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __emplace_back_result_t emplace_back(_Args&&... __args) {
----------------
ldionne wrote:

This function was previously written using `__if_likely_else` on purpose (to workaround something really subtle with Clang), see the discussion in #94379. Unless the underlying Clang issue has been fixed, this refactoring would unfortunately introduce a codegen regression. Since this is a very important function for `vector` (arguably the most important one?), I think we should go back to using it.

Now, regarding whether we can avoid setting `__end = __emplace_back_slow_path(std::forward<_Args>(__args)...);` from this function, I think we'd want to check the codegen before and after. IIRC, the idea was to make the flow of data explicitly visible to the compiler because it allowed it to generate better code (that's why we didn't call `back()` again in the pre-refactor version).

We're on the same page that the new code is better in the sense of readability and reuse, but we had decided to make a conscious readability tradeoff since this function is very performance sensitive.

Now, if we find out that we can avoid the `_end = __emplace_back_slow_path(...)` without hurting code generation (I'd be surprised because something would have had to change), that means we can avoid bifurcating `__emplace_back`. Otherwise, I'd bifurcate it for the size/pointer based vector layouts and accept it as tech debt.

https://github.com/llvm/llvm-project/pull/155330