[llvm] r178332 - Fix allocations of SmallVector and SmallPtrSet so they are more prone to
Rafael Espíndola
rafael.espindola at gmail.com
Fri Mar 29 00:13:01 PDT 2013
This broke the bots, so I have reverted it.
On 28 March 2013 22:45, Jean-Luc Duprat <jduprat at apple.com> wrote:
> Author: jduprat
> Date: Fri Mar 29 00:45:22 2013
> New Revision: 178332
>
> URL: http://llvm.org/viewvc/llvm-project?rev=178332&view=rev
> Log:
> Fix allocations of SmallVector and SmallPtrSet so they are more prone to
> being power-of-two sized.
>
>
> Modified:
> llvm/trunk/include/llvm/ADT/SmallPtrSet.h
> llvm/trunk/include/llvm/ADT/SmallVector.h
> llvm/trunk/lib/Support/SmallPtrSet.cpp
> llvm/trunk/unittests/ADT/SmallPtrSetTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/SmallPtrSet.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=178332&r1=178331&r2=178332&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallPtrSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallPtrSet.h Fri Mar 29 00:45:22 2013
> @@ -54,8 +54,6 @@ protected:
> /// then the set is in 'small mode'.
> const void **CurArray;
> /// CurArraySize - The allocated size of CurArray, always a power of two.
> - /// Note that CurArray points to an array that has CurArraySize+1 elements in
> - /// it, so that the end iterator actually points to valid memory.
> unsigned CurArraySize;
>
> // If small, this is # elts allocated consecutively
> @@ -68,9 +66,6 @@ protected:
> SmallArray(SmallStorage), CurArray(SmallStorage), CurArraySize(SmallSize) {
> assert(SmallSize && (SmallSize & (SmallSize-1)) == 0 &&
> "Initial size must be a power of two!");
> - // The end pointer, always valid, is set to a valid element to help the
> - // iterator.
> - CurArray[SmallSize] = 0;
> clear();
> }
> ~SmallPtrSetImpl();
> @@ -147,9 +142,11 @@ protected:
> class SmallPtrSetIteratorImpl {
> protected:
> const void *const *Bucket;
> + const void *const *End;
> public:
> - explicit SmallPtrSetIteratorImpl(const void *const *BP) : Bucket(BP) {
> - AdvanceIfNotValid();
> + explicit SmallPtrSetIteratorImpl(const void *const *BP, const void*const *E)
> + : Bucket(BP), End(E) {
> + AdvanceIfNotValid();
> }
>
> bool operator==(const SmallPtrSetIteratorImpl &RHS) const {
> @@ -164,8 +161,10 @@ protected:
> /// that is. This is guaranteed to stop because the end() bucket is marked
> /// valid.
> void AdvanceIfNotValid() {
> - while (*Bucket == SmallPtrSetImpl::getEmptyMarker() ||
> - *Bucket == SmallPtrSetImpl::getTombstoneMarker())
> + assert(Bucket <= End);
> + while (Bucket != End &&
> + (*Bucket == SmallPtrSetImpl::getEmptyMarker() ||
> + *Bucket == SmallPtrSetImpl::getTombstoneMarker()))
> ++Bucket;
> }
> };
> @@ -182,12 +181,13 @@ public:
> typedef std::ptrdiff_t difference_type;
> typedef std::forward_iterator_tag iterator_category;
>
> - explicit SmallPtrSetIterator(const void *const *BP)
> - : SmallPtrSetIteratorImpl(BP) {}
> + explicit SmallPtrSetIterator(const void *const *BP, const void *const *E)
> + : SmallPtrSetIteratorImpl(BP, E) {}
>
> // Most methods provided by baseclass.
>
> const PtrTy operator*() const {
> + assert(Bucket < End);
> return PtrTraits::getFromVoidPointer(const_cast<void*>(*Bucket));
> }
>
> @@ -236,9 +236,8 @@ template<class PtrType, unsigned SmallSi
> class SmallPtrSet : public SmallPtrSetImpl {
> // Make sure that SmallSize is a power of two, round up if not.
> enum { SmallSizePowTwo = RoundUpToPowerOfTwo<SmallSize>::Val };
> - /// SmallStorage - Fixed size storage used in 'small mode'. The extra element
> - /// ensures that the end iterator actually points to valid memory.
> - const void *SmallStorage[SmallSizePowTwo+1];
> + /// SmallStorage - Fixed size storage used in 'small mode'.
> + const void *SmallStorage[SmallSizePowTwo];
> typedef PointerLikeTypeTraits<PtrType> PtrTraits;
> public:
> SmallPtrSet() : SmallPtrSetImpl(SmallStorage, SmallSizePowTwo) {}
> @@ -275,10 +274,10 @@ public:
> typedef SmallPtrSetIterator<PtrType> iterator;
> typedef SmallPtrSetIterator<PtrType> const_iterator;
> inline iterator begin() const {
> - return iterator(CurArray);
> + return iterator(CurArray, CurArray+CurArraySize);
> }
> inline iterator end() const {
> - return iterator(CurArray+CurArraySize);
> + return iterator(CurArray+CurArraySize, CurArray+CurArraySize);
> }
>
> // Allow assignment from any smallptrset with the same element type even if it
>
> Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=178332&r1=178331&r2=178332&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallVector.h Fri Mar 29 00:45:22 2013
> @@ -16,6 +16,7 @@
>
> #include "llvm/Support/AlignOf.h"
> #include "llvm/Support/Compiler.h"
> +#include "llvm/Support/MathExtras.h"
> #include "llvm/Support/type_traits.h"
> #include <algorithm>
> #include <cassert>
> @@ -267,7 +268,8 @@ template <typename T, bool isPodLike>
> void SmallVectorTemplateBase<T, isPodLike>::grow(size_t MinSize) {
> size_t CurCapacity = this->capacity();
> size_t CurSize = this->size();
> - size_t NewCapacity = 2*CurCapacity + 1; // Always grow, even from zero.
> + // Always grow, even from zero.
> + size_t NewCapacity = NextPowerOf2(2*CurCapacity+(CurCapacity==0));
> if (NewCapacity < MinSize)
> NewCapacity = MinSize;
> T *NewElts = static_cast<T*>(malloc(NewCapacity*sizeof(T)));
>
> Modified: llvm/trunk/lib/Support/SmallPtrSet.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/SmallPtrSet.cpp?rev=178332&r1=178331&r2=178332&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/SmallPtrSet.cpp (original)
> +++ llvm/trunk/lib/Support/SmallPtrSet.cpp Fri Mar 29 00:45:22 2013
> @@ -29,13 +29,9 @@ void SmallPtrSetImpl::shrink_and_clear()
> NumElements = NumTombstones = 0;
>
> // Install the new array. Clear all the buckets to empty.
> - CurArray = (const void**)malloc(sizeof(void*) * (CurArraySize+1));
> + CurArray = (const void**)malloc(sizeof(void*) * CurArraySize);
> assert(CurArray && "Failed to allocate memory?");
> memset(CurArray, -1, CurArraySize*sizeof(void*));
> -
> - // The end pointer, always valid, is set to a valid element to help the
> - // iterator.
> - CurArray[CurArraySize] = 0;
> }
>
> bool SmallPtrSetImpl::insert_imp(const void * Ptr) {
> @@ -139,15 +135,11 @@ void SmallPtrSetImpl::Grow(unsigned NewS
> bool WasSmall = isSmall();
>
> // Install the new array. Clear all the buckets to empty.
> - CurArray = (const void**)malloc(sizeof(void*) * (NewSize+1));
> + CurArray = (const void**)malloc(sizeof(void*) * NewSize);
> assert(CurArray && "Failed to allocate memory?");
> CurArraySize = NewSize;
> memset(CurArray, -1, NewSize*sizeof(void*));
>
> - // The end pointer, always valid, is set to a valid element to help the
> - // iterator.
> - CurArray[NewSize] = 0;
> -
> // Copy over all the elements.
> if (WasSmall) {
> // Small sets store their elements in order.
> @@ -180,7 +172,7 @@ SmallPtrSetImpl::SmallPtrSetImpl(const v
> CurArray = SmallArray;
> // Otherwise, allocate new heap space (unless we were the same size)
> } else {
> - CurArray = (const void**)malloc(sizeof(void*) * (that.CurArraySize+1));
> + CurArray = (const void**)malloc(sizeof(void*) * that.CurArraySize);
> assert(CurArray && "Failed to allocate memory?");
> }
>
> @@ -188,7 +180,7 @@ SmallPtrSetImpl::SmallPtrSetImpl(const v
> CurArraySize = that.CurArraySize;
>
> // Copy over the contents from the other set
> - memcpy(CurArray, that.CurArray, sizeof(void*)*(CurArraySize+1));
> + memcpy(CurArray, that.CurArray, sizeof(void*)*CurArraySize);
>
> NumElements = that.NumElements;
> NumTombstones = that.NumTombstones;
> @@ -200,7 +192,7 @@ void SmallPtrSetImpl::CopyFrom(const Sma
> if (isSmall() && RHS.isSmall())
> assert(CurArraySize == RHS.CurArraySize &&
> "Cannot assign sets with different small sizes");
> -
> +
> // If we're becoming small, prepare to insert into our stack space
> if (RHS.isSmall()) {
> if (!isSmall())
> @@ -209,9 +201,9 @@ void SmallPtrSetImpl::CopyFrom(const Sma
> // Otherwise, allocate new heap space (unless we were the same size)
> } else if (CurArraySize != RHS.CurArraySize) {
> if (isSmall())
> - CurArray = (const void**)malloc(sizeof(void*) * (RHS.CurArraySize+1));
> + CurArray = (const void**)malloc(sizeof(void*) * RHS.CurArraySize);
> else
> - CurArray = (const void**)realloc(CurArray, sizeof(void*)*(RHS.CurArraySize+1));
> + CurArray = (const void**)realloc(CurArray, sizeof(void*)*RHS.CurArraySize);
> assert(CurArray && "Failed to allocate memory?");
> }
>
> @@ -219,7 +211,7 @@ void SmallPtrSetImpl::CopyFrom(const Sma
> CurArraySize = RHS.CurArraySize;
>
> // Copy over the contents from the other set
> - memcpy(CurArray, RHS.CurArray, sizeof(void*)*(CurArraySize+1));
> + memcpy(CurArray, RHS.CurArray, sizeof(void*)*CurArraySize);
>
> NumElements = RHS.NumElements;
> NumTombstones = RHS.NumTombstones;
>
> Modified: llvm/trunk/unittests/ADT/SmallPtrSetTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallPtrSetTest.cpp?rev=178332&r1=178331&r2=178332&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/SmallPtrSetTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/SmallPtrSetTest.cpp Fri Mar 29 00:45:22 2013
> @@ -17,6 +17,61 @@
> using namespace llvm;
>
> // SmallPtrSet swapping test.
> +TEST(SmallPtrSetTest, GrowthTest) {
> + int i;
> + int buf[8];
> + for(i=0; i<8; ++i) buf[i]=0;
> +
> +
> + SmallPtrSet<int *, 4> s;
> + typedef SmallPtrSet<int *, 4>::iterator iter;
> +
> + s.insert(&buf[0]);
> + s.insert(&buf[1]);
> + s.insert(&buf[2]);
> + s.insert(&buf[3]);
> + EXPECT_EQ(4U, s.size());
> +
> + i = 0;
> + for(iter I=s.begin(), E=s.end(); I!=E; ++I, ++i)
> + (**I)++;
> + EXPECT_EQ(4, i);
> + for(i=0; i<8; ++i)
> + EXPECT_EQ(i<4?1:0,buf[i]);
> +
> + s.insert(&buf[4]);
> + s.insert(&buf[5]);
> + s.insert(&buf[6]);
> + s.insert(&buf[7]);
> +
> + i = 0;
> + for(iter I=s.begin(), E=s.end(); I!=E; ++I, ++i)
> + (**I)++;
> + EXPECT_EQ(8, i);
> + s.erase(&buf[4]);
> + s.erase(&buf[5]);
> + s.erase(&buf[6]);
> + s.erase(&buf[7]);
> + EXPECT_EQ(4U, s.size());
> +
> + i = 0;
> + for(iter I=s.begin(), E=s.end(); I!=E; ++I, ++i)
> + (**I)++;
> + EXPECT_EQ(4, i);
> + for(i=0; i<8; ++i)
> + EXPECT_EQ(i<4?3:1,buf[i]);
> +
> + s.clear();
> + for(i=0; i<8; ++i) buf[i]=0;
> + for(i=0; i<128; ++i) s.insert(&buf[i%8]); // test repeated entires
> + EXPECT_EQ(8U, s.size());
> + for(iter I=s.begin(), E=s.end(); I!=E; ++I, ++i)
> + (**I)++;
> + for(i=0; i<8; ++i)
> + EXPECT_EQ(1,buf[i]);
> +}
> +
> +
> TEST(SmallPtrSetTest, SwapTest) {
> int buf[10];
>
>
>
> _______________________________________________
> 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