[libcxx-commits] [libcxx] [libc++] Assume separate storage in vector (PR #225325)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

<details>
<summary>Changes</summary>

The compiler doesn't know in general that `this` and `this->__begin_` don't alias. Since we do know that `operator new`'s returned pointers don't alias anything else we can at least inform the compiler that they don't alias if we have a `std::allocator`.

Fixes #<!-- -->181249

---
Full diff: https://github.com/llvm/llvm-project/pull/225325.diff


4 Files Affected:

- (modified) libcxx/include/__utility/assume.h (+9) 
- (modified) libcxx/include/__vector/layout.h (+9) 
- (modified) libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h (+19) 
- (modified) libcxx/test/benchmarks/containers/sequence/vector.bench.cpp (+1) 


``````````diff
diff --git a/libcxx/include/__utility/assume.h b/libcxx/include/__utility/assume.h
index 72a5efbca7b50a..6e979054527575 100644
--- a/libcxx/include/__utility/assume.h
+++ b/libcxx/include/__utility/assume.h
@@ -10,6 +10,7 @@
 #define _LIBCPP___UTILITY_ASSUME_H
 
 #include <__config>
+#include <__type_traits/is_constant_evaluated.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -24,6 +25,14 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // See https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 for a discussion.
 
 _LIBCPP_HIDE_FROM_ABI inline _LIBCPP_CONSTEXPR_SINCE_CXX14 void __assume(bool __cond) { [[__assume__(__cond)]]; }
+_LIBCPP_HIDE_FROM_ABI inline _LIBCPP_CONSTEXPR_SINCE_CXX14 void
+__assume_separate_storage([[__maybe_unused__]] const void* __a, [[__maybe_unused__]] const void* __b) {
+#if __has_builtin(__builtin_assume_separate_storage)
+  // __builtin_assume_separate_storage claims constexpr support, but it's broken. See https://llvm.org/PR225335.
+  if (!__libcpp_is_constant_evaluated())
+    __builtin_assume_separate_storage(__a, __b);
+#endif
+}
 
 _LIBCPP_END_NAMESPACE_STD
 
diff --git a/libcxx/include/__vector/layout.h b/libcxx/include/__vector/layout.h
index af03556dc2636d..d03704e4e75ab5 100644
--- a/libcxx/include/__vector/layout.h
+++ b/libcxx/include/__vector/layout.h
@@ -19,6 +19,7 @@
 #include <__memory/uninitialized_algorithms.h>
 #include <__split_buffer>
 #include <__type_traits/is_nothrow_constructible.h>
+#include <__utility/assume.h>
 #include <__utility/exchange.h>
 #include <__utility/move.h>
 #include <__utility/swap.h>
@@ -159,19 +160,27 @@ class __vector_layout {
   /// `__begin_ptr()` is not called `data()` because `vector::data()` returns `T*`, but `__begin_`
   /// is allowed to be a fancy pointer.
   [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __begin_ptr() _NOEXCEPT {
+    if _LIBCPP_CONSTEXPR (__is_std_allocator_v<allocator_type>)
+      std::__assume_separate_storage(this, __begin_);
     return __begin_;
   }
 
   [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_pointer __begin_ptr() const _NOEXCEPT {
+    if _LIBCPP_CONSTEXPR (__is_std_allocator_v<allocator_type>)
+      std::__assume_separate_storage(this, __begin_);
     return __begin_;
   }
 
   /// Returns a built-in pointer to the beginning of the buffer.
   [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _Tp* __data() _NOEXCEPT {
+    if _LIBCPP_CONSTEXPR (__is_std_allocator_v<allocator_type>)
+      std::__assume_separate_storage(this, __begin_);
     return std::__to_address(__begin_);
   }
 
   [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _Tp const* __data() const _NOEXCEPT {
+    if _LIBCPP_CONSTEXPR (__is_std_allocator_v<allocator_type>)
+      std::__assume_separate_storage(this, __begin_);
     return std::__to_address(__begin_);
   }
 
diff --git a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
index 07956ec0bea95d..8e4189a03ae6b0 100644
--- a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
+++ b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
@@ -612,6 +612,25 @@ void sequence_container_benchmarks(std::string container) {
           c.insert(c.end(), value); // re-insert an element at the end to avoid needing a new container
         }
       });
+
+    if constexpr (std::is_integral_v<ValueType>) {
+      for (auto gen : generators)
+        bench("index-loop" + tostr(gen), [gen](auto& st) TEST_ALIGN_BENCHMARK {
+          const auto size = st.range(0);
+          std::vector<ValueType> in;
+          std::generate_n(std::back_inserter(in), size, gen);
+          DoNotOptimizeData(in);
+
+          Container c(in.begin(), in.end());
+          DoNotOptimize(c);
+
+          for (auto _ : st) {
+            for (size_t i = 0; i != c.size(); ++i)
+              c[i] = std::make_unsigned_t<ValueType>(c[i]) + 1;
+            DoNotOptimize(c);
+          }
+        });
+    }
   }
 }
 
diff --git a/libcxx/test/benchmarks/containers/sequence/vector.bench.cpp b/libcxx/test/benchmarks/containers/sequence/vector.bench.cpp
index a62a75b68b1bfa..e9b0f23dc14738 100644
--- a/libcxx/test/benchmarks/containers/sequence/vector.bench.cpp
+++ b/libcxx/test/benchmarks/containers/sequence/vector.bench.cpp
@@ -27,6 +27,7 @@ struct Generate<NothrowMoveConstructible> {
 };
 
 int main(int argc, char** argv) {
+  support::sequence_container_benchmarks<std::vector<char>>("std::vector<char>");
   support::sequence_container_benchmarks<std::vector<int>>("std::vector<int>");
   support::sequence_container_benchmarks<std::vector<std::string>>("std::vector<std::string>");
   support::sequence_container_benchmarks<std::vector<NothrowMoveConstructible>>(

``````````

</details>


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