[libcxx-commits] [PATCH] D82111: Optimize 'construct at end' loops in vector

[Martijn Vels via Phabricator via libcxx-commits]

mvels created this revision.
mvels added a reviewer: EricWF.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.
EricWF accepted this revision.
This revision is now accepted and ready to land.

This change adds local 'end' and 'pos' variables for the main loop inmstead of using the ConstructTransaction variables directly.

We observed that not all vector initialization and resize operations got properly vectorized, i.e., (partially) unrolled into XMM stores for floats.

For example, `vector<int32_t> v(n, 1)` gets vectorized, but `vector<float> v(n, 1)`. It looks like the compiler assumes the state is leaked / aliased in the latter case (unclear how/why for float, but not for int32), and because of this fails to see vectorization optimization?

See https://gcc.godbolt.org/z/UWhiie

By using a local `__new_end_` (fixed), and local `__pos` (copied into __tx.__pos_ per iteration), we offer the compiler a clean loop for unrolling.

A demonstration can be seen in the isolated logic in https://gcc.godbolt.org/z/KoCNWv

The com


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82111

Files:
  libcxx/include/vector


Index: libcxx/include/vector
===================================================================
--- libcxx/include/vector
+++ libcxx/include/vector
@@ -1043,8 +1043,9 @@
 vector<_Tp, _Allocator>::__construct_at_end(size_type __n)
 {
     _ConstructTransaction __tx(*this, __n);
-    for (; __tx.__pos_ != __tx.__new_end_; ++__tx.__pos_) {
-      __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__tx.__pos_));
+    const_pointer __new_end = __tx.__new_end_;
+    for (pointer __pos = __tx.__pos_; __pos != __new_end; ++__pos, __tx.__pos_ = __pos) {
+        __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__pos));
     }
 }
 
@@ -1060,8 +1061,9 @@
 vector<_Tp, _Allocator>::__construct_at_end(size_type __n, const_reference __x)
 {
     _ConstructTransaction __tx(*this, __n);
-    for (; __tx.__pos_ != __tx.__new_end_; ++__tx.__pos_) {
-        __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__tx.__pos_), __x);
+    const_pointer __new_end = __tx.__new_end_;
+    for (pointer __pos = __tx.__pos_; __pos != __new_end; ++__pos, __tx.__pos_ = __pos) {
+        __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__pos), __x);
     }
 }
 
@@ -1753,9 +1755,10 @@
     {
       pointer __i = __from_s + __n;
       _ConstructTransaction __tx(*this, __from_e - __i);
-      for (; __i < __from_e; ++__i, ++__tx.__pos_) {
+      for (pointer __pos = __tx.__pos_; __i < __from_e;
+           ++__i, ++__pos, __tx.__pos_ = __pos) {
           __alloc_traits::construct(this->__alloc(),
-                                    _VSTD::__to_address(__tx.__pos_),
+                                    _VSTD::__to_address(__pos),
                                     _VSTD::move(*__i));
       }
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82111.271771.patch
Type: text/x-patch
Size: 1748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200618/ecb307f4/attachment-0001.bin>