[llvm-commits] [llvm] r155979 - in /llvm/trunk/include/llvm: ADT/SmallVector.h Support/Compiler.h
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Fri May 27 12:13:52 PDT 2016
On Thu, May 26, 2016 at 4:31 PM, John McCall <rjmccall at apple.com> wrote:
> > 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.
>
Oh, no worries - just figured since they were named exactly the same as
standard algorithms they might've been modelled off them & avoided using
the originals for some explicit reason, but I realize their names are
really close to other pre-c++11 standard library functions
(copy/copy_backwards) so perhaps independently derived.
Anyway, made a small change in r271036.
Also discovered Clang's __is_trivially_copyable might be a bit broken...
which might've been the original issue I was seeing from someone's issue on
IRC.
- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160527/c71cb0d2/attachment.html>
More information about the llvm-commits
mailing list