[libc++] Add assumption for align of begin and end pointers of vector. #108961

fhahn · 2024-09-17T11:14:04Z

Missing information about begin and end pointers of std::vector can lead to missed optimizations in LLVM.

This patch adds alignment assumptions at the point where the begin and end pointers are loaded. If the pointers would not have the same alignment, end might never get hit when incrementing begin.

See #101372 for a discussion of missed range check optimizations in hardened mode.

Once #108958 lands, the created llvm.assume calls for the alignment should be folded into the load instructions, resulting in no extra instructions after InstCombine.

llvmbot · 2024-09-17T11:14:39Z

@llvm/pr-subscribers-libcxx

Author: Florian Hahn (fhahn)

Changes

Missing information about begin and end pointers of std::vector can lead to missed optimizations in LLVM.

See #101372 for a discussion of missed range check optimizations in hardened mode.

Once #108958 lands, the created llvm.assume calls for the alignment should be folded into the load instructions, resulting in no extra instructions after InstCombine.

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

1 Files Affected:

(modified) libcxx/include/vector (+8-4)

diff --git a/libcxx/include/vector b/libcxx/include/vector
index 7d3aac5989a48c..720b46d573c954 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1027,6 +1027,10 @@ private:
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __move_assign_alloc(vector&, false_type) _NOEXCEPT {}
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __add_alignment_assumption(pointer __p) _NOEXCEPT {
+    return static_cast<pointer>(__builtin_assume_aligned(__p, alignof(decltype(*__p))));
+  }
 };
 
 #if _LIBCPP_STD_VER >= 17
@@ -1418,25 +1422,25 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<_Tp, _Allocator>::assign(size_type __n
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::begin() _NOEXCEPT {
-  return __make_iter(this->__begin_);
+  return __make_iter(__add_alignment_assumption(this->__begin_));
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::const_iterator
 vector<_Tp, _Allocator>::begin() const _NOEXCEPT {
-  return __make_iter(this->__begin_);
+  return __make_iter(__add_alignment_assumption(this->__begin_));
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::end() _NOEXCEPT {
-  return __make_iter(this->__end_);
+  return __make_iter(__add_alignment_assumption(this->__end_));
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::const_iterator
 vector<_Tp, _Allocator>::end() const _NOEXCEPT {
-  return __make_iter(this->__end_);
+  return __make_iter(__add_alignment_assumption(this->__end_));
 }
 
 template <class _Tp, class _Allocator>

philnik777 · 2024-09-17T11:18:54Z

libcxx/include/vector

+
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __add_alignment_assumption(pointer __p) _NOEXCEPT {
+    return static_cast<pointer>(__builtin_assume_aligned(__p, alignof(decltype(*__p))));
+  }


Does this work with fancy pointers? Also, shouldn't the compiler be able to infer this?

libcxx/include/vector

Missing information about begin and end pointers of std::vector can lead to missed optimizations in LLVM. See llvm#101372 for a discussion of missed range check optimizations in hardened mode. Once llvm#108958 lands, the created `llvm.assume` calls for the alignment should be folded into the `load` instructions, resulting in no extra instructions after InstCombine.

Otherwise, this prevents us from being able to declare a flat_map containing an incomplete type.

ldionne · 2025-01-13T20:20:26Z

libcxx/include/__flat_map/key_value_iterator.h

@@ -15,9 +15,7 @@
 #include <__config>


@huixie90 Do these changes look correct to you? Is there a reason why you used ranges::iterator_t in the first place?

Use known bits to remove redundant alignment assumptions. Libc++ now adds alignment assumptions for std::vector::begin() and std::vector::end(), so I expect we will see quite a bit more assumptions in C++ [1]. Try to clean up some redundant ones to start with. [1] llvm#108961

zmodem · 2025-01-28T18:37:22Z

libcxx/include/__vector/vector.h

@@ -775,6 +780,17 @@ class _LIBCPP_TEMPLATE_VIS vector {
  }

  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __move_assign_alloc(vector&, false_type) _NOEXCEPT {}
+
+  static _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __add_alignment_assumption(pointer __p) _NOEXCEPT {


Similarly to #118837 (review) I think this one needs _LIBCPP_NO_CFI since we're potentially static_cast'ing uninitialized memory (for the end pointer).

(Apologies for the late notice, we're a bit behind on libc++.)

Ack. I'd be curious to see how this failure can be reproduced. Is it just a matter of adding a -fsanitize=cfi job to our pre-commit CI?

CF #124837

Also #124839 for adding NO_CFI

I haven't tried to reproduce it on the libc++ tests directly, but it should be possible. I think we're currently only using a subset of the cfi checks: -fsanitize=cfi-vcall -fsanitize=cfi-derived-cast -fsanitize=cfi-unrelated-cast. The annoying thing is that cfi requires (thin) lto.

winner245 · 2025-01-28T19:32:29Z

libcxx/include/__vector/vector.h

+#ifndef _LIBCPP_CXX03_LANG
+    if constexpr (is_pointer<pointer>::value) {


@ldionne Are we using constexpr if in C++11/14 as a compiler extension here? I've also noticed several other usages of constexpr if in C++14 mode in libc++.
Does this imply that we can generally use constexpr if within the context of C++11/14 in libc++?

Yes, it's fine to use compiler extensions in the library as long as our supported compilers implement them.

fhahn requested a review from a team as a code owner September 17, 2024 11:14

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 17, 2024

fhahn mentioned this pull request Sep 17, 2024

Loop invariant is not deduced in C++-iterator-style loop over pointers #101372

Open

philnik777 reviewed Sep 17, 2024

View reviewed changes

danakj reviewed Sep 17, 2024

View reviewed changes

libcxx/include/vector Outdated Show resolved Hide resolved

fhahn mentioned this pull request Sep 17, 2024

[InstCombine] Fold align assume into load's !align metadata if possible. #108958

Open

ldionne reviewed Sep 19, 2024

View reviewed changes

libcxx/include/vector Outdated Show resolved Hide resolved

libcxx/include/vector Outdated Show resolved Hide resolved

libcxx/include/vector Outdated Show resolved Hide resolved

ldionne added the performance label Nov 28, 2024

ldionne force-pushed the libcxx-vector-begin-end-align branch from 83f60da to 94c2ede Compare January 13, 2025 17:37

Address review comments

3daa6c5

ldionne approved these changes Jan 13, 2025

View reviewed changes

ldionne added the pending-ci Merging the PR is only pending completion of CI label Jan 13, 2025

ldionne added 3 commits January 13, 2025 14:14

Handle C++03

ddb387b

Add missing include

a2cd12c

Avoid instantiating vector::begin() when declaring flat_map

a9fb6aa

Otherwise, this prevents us from being able to declare a flat_map containing an incomplete type.

ldionne reviewed Jan 13, 2025

View reviewed changes

ldionne merged commit eac23a5 into llvm:main Jan 16, 2025
81 checks passed

fhahn mentioned this pull request Jan 17, 2025

[InstCombine] Remove redundant alignment assumptions. #123348

Open

zmodem reviewed Jan 28, 2025

View reviewed changes

winner245 reviewed Jan 28, 2025

View reviewed changes

fhahn deleted the libcxx-vector-begin-end-align branch February 25, 2025 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Add assumption for align of begin and end pointers of vector. #108961

[libc++] Add assumption for align of begin and end pointers of vector. #108961

Uh oh!

fhahn commented Sep 17, 2024 •

edited

Loading

Uh oh!

llvmbot commented Sep 17, 2024

Uh oh!

philnik777 Sep 17, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne Jan 13, 2025

Uh oh!

Uh oh!

zmodem Jan 28, 2025

Uh oh!

ldionne Jan 28, 2025 •

edited

Loading

Uh oh!

ldionne Jan 28, 2025

Uh oh!

zmodem Jan 29, 2025

Uh oh!

winner245 Jan 28, 2025

Uh oh!

ldionne Jan 28, 2025

Uh oh!

Uh oh!

		#ifndef _LIBCPP_CXX03_LANG
		if constexpr (is_pointer<pointer>::value) {

[libc++] Add assumption for align of begin and end pointers of vector. #108961

[libc++] Add assumption for align of begin and end pointers of vector. #108961

Uh oh!

Conversation

fhahn commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 17, 2024

Uh oh!

philnik777 Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zmodem Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

ldionne Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldionne Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

zmodem Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

winner245 Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

ldionne Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fhahn commented Sep 17, 2024 •

edited

Loading

ldionne Jan 28, 2025 •

edited

Loading