[llvm-commits] [llvm] r161041 - in /llvm/trunk: include/llvm/ADT/TinyPtrVector.h unittests/ADT/TinyPtrVectorTest.cpp

David Blaikie dblaikie at gmail.com
Tue Jul 31 08:15:58 PDT 2012


On Tue, Jul 31, 2012 at 2:42 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Tue Jul 31 04:42:24 2012
> New Revision: 161041
>
> URL: http://llvm.org/viewvc/llvm-project?rev=161041&view=rev
> Log:
> Implement copy and move assignment for TinyPtrVector. These try to
> re-use allocated vectors as much as possible.
>
> Modified:
>     llvm/trunk/include/llvm/ADT/TinyPtrVector.h
>     llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/TinyPtrVector.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/TinyPtrVector.h?rev=161041&r1=161040&r2=161041&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/TinyPtrVector.h (original)
> +++ llvm/trunk/include/llvm/ADT/TinyPtrVector.h Tue Jul 31 04:42:24 2012
> @@ -32,19 +32,72 @@
>    llvm::PointerUnion<EltTy, VecTy*> Val;
>
>    TinyPtrVector() {}
> +  ~TinyPtrVector() {
> +    if (VecTy *V = Val.template dyn_cast<VecTy*>())
> +      delete V;
> +  }
> +
>    TinyPtrVector(const TinyPtrVector &RHS) : Val(RHS.Val) {
>      if (VecTy *V = Val.template dyn_cast<VecTy*>())
>        Val = new VecTy(*V);
>    }
> +  TinyPtrVector &operator=(const TinyPtrVector &RHS) {
> +    if (this == &RHS)
> +      return *this;
> +    if (RHS.empty()) {
> +      this->clear();
> +      return *this;
> +    }
> +
> +    // Try to squeeze into the single slot. If it won't fit, allocate a copied
> +    // vector.
> +    if (Val.template is<EltTy>()) {
> +      if (RHS.size() == 1)
> +        Val = RHS.front();
> +      else
> +        Val = new VecTy(*RHS.Val.template get<VecTy*>());
> +      return *this;
> +    }
> +
> +    // If we have a full vector allocated, try to re-use it.
> +    if (RHS.Val.template is<EltTy>()) {
> +      Val.template get<VecTy*>()->clear();
> +      Val.template get<VecTy*>()->push_back(RHS.front());
> +    } else {
> +      *Val.template get<VecTy*>() = *RHS.Val.template get<VecTy*>();
> +    }
> +    return *this;
> +  }
> +
>  #if LLVM_USE_RVALUE_REFERENCES
>    TinyPtrVector(TinyPtrVector &&RHS) : Val(RHS.Val) {
>      RHS.Val = (EltTy)0;
>    }
> -#endif
> -  ~TinyPtrVector() {
> -    if (VecTy *V = Val.template dyn_cast<VecTy*>())
> +  TinyPtrVector &operator=(TinyPtrVector &&RHS) {

Is there a reason you preferred an explicit move assignment rather
than a unified one ("unified" being just having one op=, that takes
its argument by value (& has the implementation of your current move
assignment operator))

(I guess the fact that this type has a relatively expensive move
construction might mean it's worthwhile to write these two functions
out separately rather than relying on the unified one (which incurs an
extra move in each case))

> +    if (this == &RHS)
> +      return *this;
> +    if (RHS.empty()) {
> +      this->clear();
> +      return *this;
> +    }
> +
> +    // If this vector has been allocated on the heap, re-use it if cheap. If it
> +    // would require more copying, just delete it and we'll steal the other
> +    // side.
> +    if (VecTy *V = Val.template dyn_cast<VecTy*>()) {
> +      if (RHS.Val.template is<EltTy>()) {
> +        V->clear();
> +        V->push_back(RHS.front());
> +        return *this;
> +      }
>        delete V;
> +    }
> +
> +    Val = RHS.Val;
> +    RHS.Val = (EltTy)0;
> +    return *this;
>    }
> +#endif
>
>    // implicit conversion operator to ArrayRef.
>    operator ArrayRef<EltTy>() const {
> @@ -173,12 +226,6 @@
>      }
>      return end();
>    }
> -
> -private:
> -  void operator=(const TinyPtrVector&); // NOT IMPLEMENTED YET.
> -#if LLVM_USE_RVALUE_REFERENCES
> -  void operator=(TinyPtrVector&&); // NOT IMPLEMENTED YET.
> -#endif
>  };
>  } // end namespace llvm
>
>
> Modified: llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp?rev=161041&r1=161040&r2=161041&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp Tue Jul 31 04:42:24 2012
> @@ -60,6 +60,13 @@
>        V.push_back(Values[i]);
>    }
>
> +  void setVectors(ArrayRef<PtrT> Values1, ArrayRef<PtrT> Values2) {
> +    V.clear();
> +    appendValues(V, Values1);
> +    V2.clear();
> +    appendValues(V2, Values2);
> +  }
> +
>    void expectValues(const VectorT &V, ArrayRef<PtrT> Values) {
>      EXPECT_EQ(Values.empty(), V.empty());
>      EXPECT_EQ(Values.size(), V.size());
> @@ -157,6 +164,165 @@
>  #endif
>  }
>
> +TYPED_TEST(TinyPtrVectorTest, CopyAndMoveTest) {
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(0));
> +  this->expectValues(this->V2, this->testArray(0));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(0));
> +#endif
> +
> +  this->setVectors(this->testArray(1), this->testArray(0));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(0));
> +  this->expectValues(this->V2, this->testArray(0));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(1), this->testArray(0));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(0));
> +#endif
> +
> +  this->setVectors(this->testArray(2), this->testArray(0));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(0));
> +  this->expectValues(this->V2, this->testArray(0));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(2), this->testArray(0));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(0));
> +#endif
> +
> +  this->setVectors(this->testArray(42), this->testArray(0));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(0));
> +  this->expectValues(this->V2, this->testArray(0));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(42), this->testArray(0));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(0));
> +#endif
> +
> +  this->setVectors(this->testArray(0), this->testArray(1));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(1));
> +  this->expectValues(this->V2, this->testArray(1));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(0), this->testArray(1));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(1));
> +#endif
> +
> +  this->setVectors(this->testArray(0), this->testArray(2));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(2));
> +  this->expectValues(this->V2, this->testArray(2));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(0), this->testArray(2));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(2));
> +#endif
> +
> +  this->setVectors(this->testArray(0), this->testArray(42));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(42));
> +  this->expectValues(this->V2, this->testArray(42));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(0), this->testArray(42));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(42));
> +#endif
> +
> +  this->setVectors(this->testArray(1), this->testArray(1));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(1));
> +  this->expectValues(this->V2, this->testArray(1));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(1));
> +#endif
> +
> +  this->setVectors(this->testArray(1), this->testArray(2));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(2));
> +  this->expectValues(this->V2, this->testArray(2));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(1), this->testArray(2));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(2));
> +#endif
> +
> +  this->setVectors(this->testArray(1), this->testArray(42));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(42));
> +  this->expectValues(this->V2, this->testArray(42));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(1), this->testArray(42));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(42));
> +#endif
> +
> +  this->setVectors(this->testArray(2), this->testArray(1));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(1));
> +  this->expectValues(this->V2, this->testArray(1));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(2), this->testArray(1));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(1));
> +#endif
> +
> +  this->setVectors(this->testArray(2), this->testArray(2));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(2));
> +  this->expectValues(this->V2, this->testArray(2));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(2), this->testArray(2));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(2));
> +#endif
> +
> +  this->setVectors(this->testArray(2), this->testArray(42));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(42));
> +  this->expectValues(this->V2, this->testArray(42));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(2), this->testArray(42));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(42));
> +#endif
> +
> +  this->setVectors(this->testArray(42), this->testArray(1));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(1));
> +  this->expectValues(this->V2, this->testArray(1));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(42), this->testArray(1));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(1));
> +#endif
> +
> +  this->setVectors(this->testArray(42), this->testArray(2));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(2));
> +  this->expectValues(this->V2, this->testArray(2));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(42), this->testArray(2));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(2));
> +#endif
> +
> +  this->setVectors(this->testArray(42), this->testArray(42));
> +  this->V = this->V2;
> +  this->expectValues(this->V, this->testArray(42));
> +  this->expectValues(this->V2, this->testArray(42));
> +#if LLVM_USE_RVALUE_REFERENCES
> +  this->setVectors(this->testArray(42), this->testArray(42));
> +  this->V = std::move(this->V2);
> +  this->expectValues(this->V, this->testArray(42));
> +#endif
> +}
> +
>  TYPED_TEST(TinyPtrVectorTest, EraseTest) {
>    this->appendValues(this->V, this->testArray(1));
>    this->expectValues(this->V, this->testArray(1));
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list