[llvm-commits] [llvm] r155979 - in /llvm/trunk/include/llvm: ADT/SmallVector.h Support/Compiler.h

Thu May 26 16:31:49 PDT 2016

> On May 26, 2016, at 4:27 PM, David Blaikie <dblaikie at gmail.com> wrote:
> /casts Necromancy
> 
> (Sorry for the duplicate, John - forgot to switch mailing lists, of course)
> 
> On Tue, May 1, 2012 at 10:39 PM, John McCall <rjmccall at apple.com> wrote:
> Author: rjmccall
> Date: Wed May  2 00:39:15 2012
> New Revision: 155979
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=155979&view=rev
> Log:
> Update SmallVector to support move semantics if the host does.
> Note that support for rvalue references does not imply support
> for the full set of move-related STL operations.
> 
> I've preserved support for an odd little thing in insert() where
> we're trying to support inserting a new element from an existing
> one.  If we actually want to support that, there's a lot more we
> need to do:  insert can call either grow or push_back, neither of
> which is safe against this particular use pattern.
> 
> Modified:
>     llvm/trunk/include/llvm/ADT/SmallVector.h
>     llvm/trunk/include/llvm/Support/Compiler.h
> 
> Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=155979&r1=155978&r2=155979&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallVector.h Wed May  2 00:39:15 2012
> @@ -14,6 +14,7 @@
>  #ifndef LLVM_ADT_SMALLVECTOR_H
>  #define LLVM_ADT_SMALLVECTOR_H
> 
> +#include "llvm/Support/Compiler.h"
>  #include "llvm/Support/type_traits.h"
>  #include <algorithm>
>  #include <cassert>
> @@ -54,6 +55,11 @@
>      return BeginX == static_cast<const void*>(&FirstEl);
>    }
> 
> +  /// resetToSmall - Put this vector in a state of being small.
> +  void resetToSmall() {
> +    BeginX = EndX = CapacityX = &FirstEl;
> +  }
> +
>    /// grow_pod - This is an implementation of the grow() method which only works
>    /// on POD-like data types and is out of line to reduce code duplication.
>    void grow_pod(size_t MinSizeInBytes, size_t TSize);
> @@ -160,28 +166,84 @@
>      }
>    }
> 
> -  /// uninitialized_copy - Copy the range [I, E) onto the uninitialized memory
> -  /// starting with "Dest", constructing elements into it as needed.
> +  /// move - Use move-assignment to move the range [I, E) onto the
> +  /// objects starting with "Dest".  This is just <memory>'s
> +  /// std::move, but not all stdlibs actually provide that.
> +  template<typename It1, typename It2>
> +  static It2 move(It1 I, It1 E, It2 Dest) {
> +#if LLVM_USE_RVALUE_REFERENCES
> +    for (; I != E; ++I, ++Dest)
> +      *Dest = ::std::move(*I);
> +    return Dest;
> +#else
> +    return ::std::copy(I, E, Dest);
> +#endif
> +  }
> +
> +  /// move_backward - Use move-assignment to move the range
> +  /// [I, E) onto the objects ending at "Dest", moving objects
> +  /// in reverse order.  This is just <algorithm>'s
> +  /// std::move_backward, but not all stdlibs actually provide that.
> +  template<typename It1, typename It2>
> +  static It2 move_backward(It1 I, It1 E, It2 Dest) {
> +#if LLVM_USE_RVALUE_REFERENCES
> +    while (I != E)
> +      *--Dest = ::std::move(*--E);
> +    return Dest;
> +#else
> +    return ::std::copy_backward(I, E, Dest);
> +#endif
> +  }
> +
> +  /// uninitialized_move - Move the range [I, E) into the uninitialized
> +  /// memory starting with "Dest", constructing elements as needed.
> +  template<typename It1, typename It2>
> +  static void uninitialized_move(It1 I, It1 E, It2 Dest) {
> +#if LLVM_USE_RVALUE_REFERENCES
> +    for (; I != E; ++I, ++Dest)
> +      ::new ((void*) &*Dest) T(::std::move(*I));
> Hey John - I was just looking over all this code & was wondering why these static move_backwards and move(iter, iter, iter) were implemented this way.
> 
> Practically speaking, what I mean is: Can I remove them all and just replace the call sites with std::move(iter, iter, iter) and std::move_backward?
> 
> (It looks like in this original version the loops were written out manually - perhaps because we were running on platforms that didn't have those library functions implemented? Then we... hmm, nope, they're still implemented explicitly in the isPodLike=false case - I assume we could just move this all to std::move/std::move_backwards & we'd be good to go?)
> +#else
> +    ::std::uninitialized_copy(I, E, Dest);
> +#endif
> +  }
> +
> +  /// uninitialized_copy - Copy the range [I, E) onto the uninitialized
> +  /// memory starting with "Dest", constructing elements as needed.
>    template<typename It1, typename It2>
>    static void uninitialized_copy(It1 I, It1 E, It2 Dest) {
>      std::uninitialized_copy(I, E, Dest);
>    }
> 
> -  /// grow - double the size of the allocated memory, guaranteeing space for at
> -  /// least one more element or MinSize if specified.
> +  /// grow - Grow the allocated memory (without initializing new
> +  /// elements), doubling the size of the allocated memory.
> +  /// Guarantees space for at least one more element, or MinSize more
> +  /// elements if specified.
>    void grow(size_t MinSize = 0);
> 
>  public:
>    void push_back(const T &Elt) {
>      if (this->EndX < this->CapacityX) {
>      Retry:
> -      new (this->end()) T(Elt);
> +      ::new ((void*) this->end()) T(Elt);
> +      this->setEnd(this->end()+1);
> +      return;
> +    }
> +    this->grow();
> +    goto Retry;
> +  }
> +
> +#if LLVM_USE_RVALUE_REFERENCES
> +  void push_back(T &&Elt) {
> +    if (this->EndX < this->CapacityX) {
> +    Retry:
> +      ::new ((void*) this->end()) T(::std::move(Elt));
>        this->setEnd(this->end()+1);
>        return;
>      }
>      this->grow();
>      goto Retry;
>    }
> +#endif
> 
>    void pop_back() {
>      this->setEnd(this->end()-1);
> @@ -199,8 +261,8 @@
>      NewCapacity = MinSize;
>    T *NewElts = static_cast<T*>(malloc(NewCapacity*sizeof(T)));
> 
> -  // Copy the elements over.
> -  this->uninitialized_copy(this->begin(), this->end(), NewElts);
> +  // Move the elements over.
> +  this->uninitialized_move(this->begin(), this->end(), NewElts);
> 
>    // Destroy the original elements.
>    destroy_range(this->begin(), this->end());
> @@ -225,6 +287,29 @@
>    // No need to do a destroy loop for POD's.
>    static void destroy_range(T *, T *) {}
> 
> +  /// move - Use move-assignment to move the range [I, E) onto the
> +  /// objects starting with "Dest".  For PODs, this is just memcpy.
> +  template<typename It1, typename It2>
> +  static It2 move(It1 I, It1 E, It2 Dest) {
> +    return ::std::copy(I, E, Dest);
> 
> Hey John - I was just looking over all this code & was wondering why these static move_backwards and move(iter, iter, iter) were implemented this way.
> 
> Practically speaking, what I mean is: Can I remove them all and just replace the call sites with std::move(iter, iter, iter) and std::move_backward?
> 
> (It looks like in this original version the loops were written out manually - perhaps because we were running on platforms that didn't have those library functions implemented? Then we... hmm, nope, they're still implemented explicitly in the isPodLike=false case - I assume we could just move this all to std::move/std::move_backwards & we'd be good to go?)

You should generally assume that I don't have an encyclopedic knowledge of the standard library. :)  If there are functions that do this reliably already, and don't do something dumb like actually use copy-assignment instead of move-assignment if they can't prove noexcept, feel free to use them.

John.