From: Duncan P. N. Exon Smith Date: Fri, 20 Jul 2018 00:09:56 +0000 (+0000) Subject: Revert "ADT: Shrink size of SmallVector by 8B on 64-bit platforms" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0ee7ae5caa2255918d2288443ea9881f91d6ea64;p=llvm Revert "ADT: Shrink size of SmallVector by 8B on 64-bit platforms" This reverts commit r337504 while I investigate a TSan bot failure that seems related: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/26526 #8 0x000055581f2895d8 (/b/sanitizer-x86_64-linux-autoconf/build/tsan_debug_build/bin/clang-7+0x1eb45d8) #9 0x000055581f294323 llvm::ConstantAggrKeyType::create(llvm::ArrayType*) const /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:409:0 #10 0x000055581f294323 llvm::ConstantUniqueMap::create(llvm::ArrayType*, llvm::ConstantAggrKeyType, std::pair > >&) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:635:0 #11 0x000055581f294323 llvm::ConstantUniqueMap::getOrCreate(llvm::ArrayType*, llvm::ConstantAggrKeyType) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:654:0 #12 0x000055581f2944cb llvm::ConstantArray::get(llvm::ArrayType*, llvm::ArrayRef) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/Constants.cpp:964:0 #13 0x000055581fa27e19 llvm::SmallVectorBase::size() const /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:53:0 #14 0x000055581fa27e19 llvm::SmallVectorImpl::resize(unsigned long) /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:347:0 #15 0x000055581fa27e19 (anonymous namespace)::EmitArrayConstant(clang::CodeGen::CodeGenModule&, clang::ConstantArrayType const*, llvm::Type*, unsigned int, llvm::SmallVectorImpl&, llvm::Constant*) /b/sanitizer-x86_64-linux-autoconf/build/llvm/tools/clang/lib/CodeGen/CGExprConstant.cpp:669:0 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337511 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ADT/SmallVector.h b/include/llvm/ADT/SmallVector.h index d9a7c6f9538..240139c298f 100644 --- a/include/llvm/ADT/SmallVector.h +++ b/include/llvm/ADT/SmallVector.h @@ -38,36 +38,28 @@ namespace llvm { /// This is all the non-templated stuff common to all SmallVectors. class SmallVectorBase { protected: - void *BeginX; - unsigned Size = 0, Capacity; + void *BeginX, *EndX, *CapacityX; - SmallVectorBase() = delete; - SmallVectorBase(void *FirstEl, size_t Capacity) - : BeginX(FirstEl), Capacity(Capacity) {} +protected: + SmallVectorBase(void *FirstEl, size_t Size) + : BeginX(FirstEl), EndX(FirstEl), CapacityX((char*)FirstEl+Size) {} /// 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(void *FirstEl, size_t MinCapacity, size_t TSize); + void grow_pod(void *FirstEl, size_t MinSizeInBytes, size_t TSize); public: - size_t size() const { return Size; } - size_t capacity() const { return Capacity; } - - LLVM_NODISCARD bool empty() const { return !Size; } + /// This returns size()*sizeof(T). + size_t size_in_bytes() const { + return size_t((char*)EndX - (char*)BeginX); + } - /// Set the array size to \p N, which the current array must have enough - /// capacity for. - /// - /// This does not construct or destroy any elements in the vector. - /// - /// Clients can use this in conjunction with capacity() to write past the end - /// of the buffer when they know that more elements are available, and only - /// update the size later. This avoids the cost of value initializing elements - /// which will only be overwritten. - void set_size(size_t Size) { - assert(Size <= capacity()); - this->Size = Size; + /// capacity_in_bytes - This returns capacity()*sizeof(T). + size_t capacity_in_bytes() const { + return size_t((char*)CapacityX - (char*)BeginX); } + + LLVM_NODISCARD bool empty() const { return BeginX == EndX; } }; /// This is the part of SmallVectorTemplateBase which does not depend on whether @@ -88,8 +80,8 @@ private: protected: SmallVectorTemplateCommon(size_t Size) : SmallVectorBase(&FirstEl, Size) {} - void grow_pod(size_t MinCapacity, size_t TSize) { - SmallVectorBase::grow_pod(&FirstEl, MinCapacity, TSize); + void grow_pod(size_t MinSizeInBytes, size_t TSize) { + SmallVectorBase::grow_pod(&FirstEl, MinSizeInBytes, TSize); } /// Return true if this is a smallvector which has not had dynamic @@ -100,10 +92,11 @@ protected: /// Put this vector in a state of being small. void resetToSmall() { - BeginX = &FirstEl; - Size = Capacity = 0; // FIXME: Setting Capacity to 0 is suspect. + BeginX = EndX = CapacityX = &FirstEl; } + void setEnd(T *P) { this->EndX = P; } + public: using size_type = size_t; using difference_type = ptrdiff_t; @@ -125,20 +118,27 @@ public: LLVM_ATTRIBUTE_ALWAYS_INLINE const_iterator begin() const { return (const_iterator)this->BeginX; } LLVM_ATTRIBUTE_ALWAYS_INLINE - iterator end() { return begin() + size(); } + iterator end() { return (iterator)this->EndX; } LLVM_ATTRIBUTE_ALWAYS_INLINE - const_iterator end() const { return begin() + size(); } + const_iterator end() const { return (const_iterator)this->EndX; } +protected: + iterator capacity_ptr() { return (iterator)this->CapacityX; } + const_iterator capacity_ptr() const { return (const_iterator)this->CapacityX;} + +public: // reverse iterator creation methods. reverse_iterator rbegin() { return reverse_iterator(end()); } const_reverse_iterator rbegin() const{ return const_reverse_iterator(end()); } reverse_iterator rend() { return reverse_iterator(begin()); } const_reverse_iterator rend() const { return const_reverse_iterator(begin());} - size_type size_in_bytes() const { return size() * sizeof(T); } + LLVM_ATTRIBUTE_ALWAYS_INLINE + size_type size() const { return end()-begin(); } size_type max_size() const { return size_type(-1) / sizeof(T); } - size_t capacity_in_bytes() const { return capacity() * sizeof(T); } + /// Return the total number of elements in the currently allocated buffer. + size_t capacity() const { return capacity_ptr() - begin(); } /// Return a pointer to the vector's buffer, even if empty(). pointer data() { return pointer(begin()); } @@ -211,21 +211,21 @@ protected: public: void push_back(const T &Elt) { - if (LLVM_UNLIKELY(this->size() >= this->capacity())) + if (LLVM_UNLIKELY(this->EndX >= this->CapacityX)) this->grow(); ::new ((void*) this->end()) T(Elt); - this->set_size(this->size() + 1); + this->setEnd(this->end()+1); } void push_back(T &&Elt) { - if (LLVM_UNLIKELY(this->size() >= this->capacity())) + if (LLVM_UNLIKELY(this->EndX >= this->CapacityX)) this->grow(); ::new ((void*) this->end()) T(::std::move(Elt)); - this->set_size(this->size() + 1); + this->setEnd(this->end()+1); } void pop_back() { - this->set_size(this->size() - 1); + this->setEnd(this->end()-1); this->end()->~T(); } }; @@ -233,12 +233,12 @@ public: // Define this out-of-line to dissuade the C++ compiler from inlining it. template void SmallVectorTemplateBase::grow(size_t MinSize) { - if (MinSize > UINT32_MAX) - report_bad_alloc_error("SmallVector capacity overflow during allocation"); - + size_t CurCapacity = this->capacity(); + size_t CurSize = this->size(); // Always grow, even from zero. - size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2)); - NewCapacity = std::min(std::max(NewCapacity, MinSize), size_t(UINT32_MAX)); + size_t NewCapacity = size_t(NextPowerOf2(CurCapacity+2)); + if (NewCapacity < MinSize) + NewCapacity = MinSize; T *NewElts = static_cast(llvm::safe_malloc(NewCapacity*sizeof(T))); // Move the elements over. @@ -251,8 +251,9 @@ void SmallVectorTemplateBase::grow(size_t MinSize) { if (!this->isSmall()) free(this->begin()); + this->setEnd(NewElts+CurSize); this->BeginX = NewElts; - this->Capacity = NewCapacity; + this->CapacityX = this->begin()+NewCapacity; } @@ -299,17 +300,21 @@ protected: /// Double the size of the allocated memory, guaranteeing space for at /// least one more element or MinSize if specified. - void grow(size_t MinSize = 0) { this->grow_pod(MinSize, sizeof(T)); } + void grow(size_t MinSize = 0) { + this->grow_pod(MinSize*sizeof(T), sizeof(T)); + } public: void push_back(const T &Elt) { - if (LLVM_UNLIKELY(this->size() >= this->capacity())) + if (LLVM_UNLIKELY(this->EndX >= this->CapacityX)) this->grow(); memcpy(this->end(), &Elt, sizeof(T)); - this->set_size(this->size() + 1); + this->setEnd(this->end()+1); } - void pop_back() { this->set_size(this->size() - 1); } + void pop_back() { + this->setEnd(this->end()-1); + } }; /// This class consists of common code factored out of the SmallVector class to @@ -326,7 +331,8 @@ public: protected: // Default ctor - Initialize to empty. explicit SmallVectorImpl(unsigned N) - : SmallVectorTemplateBase::value>(N) {} + : SmallVectorTemplateBase::value>(N*sizeof(T)) { + } public: SmallVectorImpl(const SmallVectorImpl &) = delete; @@ -340,31 +346,31 @@ public: void clear() { this->destroy_range(this->begin(), this->end()); - this->Size = 0; + this->EndX = this->BeginX; } void resize(size_type N) { if (N < this->size()) { this->destroy_range(this->begin()+N, this->end()); - this->set_size(N); + this->setEnd(this->begin()+N); } else if (N > this->size()) { if (this->capacity() < N) this->grow(N); for (auto I = this->end(), E = this->begin() + N; I != E; ++I) new (&*I) T(); - this->set_size(N); + this->setEnd(this->begin()+N); } } void resize(size_type N, const T &NV) { if (N < this->size()) { this->destroy_range(this->begin()+N, this->end()); - this->set_size(N); + this->setEnd(this->begin()+N); } else if (N > this->size()) { if (this->capacity() < N) this->grow(N); std::uninitialized_fill(this->end(), this->begin()+N, NV); - this->set_size(N); + this->setEnd(this->begin()+N); } } @@ -389,23 +395,23 @@ public: void append(in_iter in_start, in_iter in_end) { size_type NumInputs = std::distance(in_start, in_end); // Grow allocated space if needed. - if (NumInputs > this->capacity() - this->size()) + if (NumInputs > size_type(this->capacity_ptr()-this->end())) this->grow(this->size()+NumInputs); // Copy the new elements over. this->uninitialized_copy(in_start, in_end, this->end()); - this->set_size(this->size() + NumInputs); + this->setEnd(this->end() + NumInputs); } /// Add the specified range to the end of the SmallVector. void append(size_type NumInputs, const T &Elt) { // Grow allocated space if needed. - if (NumInputs > this->capacity() - this->size()) + if (NumInputs > size_type(this->capacity_ptr()-this->end())) this->grow(this->size()+NumInputs); // Copy the new elements over. std::uninitialized_fill_n(this->end(), NumInputs, Elt); - this->set_size(this->size() + NumInputs); + this->setEnd(this->end() + NumInputs); } void append(std::initializer_list IL) { @@ -419,7 +425,7 @@ public: clear(); if (this->capacity() < NumElts) this->grow(NumElts); - this->set_size(NumElts); + this->setEnd(this->begin()+NumElts); std::uninitialized_fill(this->begin(), this->end(), Elt); } @@ -466,7 +472,7 @@ public: iterator I = std::move(E, this->end(), S); // Drop the last elts. this->destroy_range(I, this->end()); - this->set_size(I - this->begin()); + this->setEnd(I); return(N); } @@ -479,7 +485,7 @@ public: assert(I >= this->begin() && "Insertion iterator is out of bounds."); assert(I <= this->end() && "Inserting past the end of the vector."); - if (this->size() >= this->capacity()) { + if (this->EndX >= this->CapacityX) { size_t EltNo = I-this->begin(); this->grow(); I = this->begin()+EltNo; @@ -488,12 +494,12 @@ public: ::new ((void*) this->end()) T(::std::move(this->back())); // Push everything else over. std::move_backward(I, this->end()-1, this->end()); - this->set_size(this->size() + 1); + this->setEnd(this->end()+1); // If we just moved the element we're inserting, be sure to update // the reference. T *EltPtr = &Elt; - if (I <= EltPtr && EltPtr < this->end()) + if (I <= EltPtr && EltPtr < this->EndX) ++EltPtr; *I = ::std::move(*EltPtr); @@ -509,7 +515,7 @@ public: assert(I >= this->begin() && "Insertion iterator is out of bounds."); assert(I <= this->end() && "Inserting past the end of the vector."); - if (this->size() >= this->capacity()) { + if (this->EndX >= this->CapacityX) { size_t EltNo = I-this->begin(); this->grow(); I = this->begin()+EltNo; @@ -517,12 +523,12 @@ public: ::new ((void*) this->end()) T(std::move(this->back())); // Push everything else over. std::move_backward(I, this->end()-1, this->end()); - this->set_size(this->size() + 1); + this->setEnd(this->end()+1); // If we just moved the element we're inserting, be sure to update // the reference. const T *EltPtr = &Elt; - if (I <= EltPtr && EltPtr < this->end()) + if (I <= EltPtr && EltPtr < this->EndX) ++EltPtr; *I = *EltPtr; @@ -568,7 +574,7 @@ public: // Move over the elements that we're about to overwrite. T *OldEnd = this->end(); - this->set_size(this->size() + NumToInsert); + this->setEnd(this->end() + NumToInsert); size_t NumOverwritten = OldEnd-I; this->uninitialized_move(I, OldEnd, this->end()-NumOverwritten); @@ -625,7 +631,7 @@ public: // Move over the elements that we're about to overwrite. T *OldEnd = this->end(); - this->set_size(this->size() + NumToInsert); + this->setEnd(this->end() + NumToInsert); size_t NumOverwritten = OldEnd-I; this->uninitialized_move(I, OldEnd, this->end()-NumOverwritten); @@ -645,10 +651,10 @@ public: } template void emplace_back(ArgTypes &&... Args) { - if (LLVM_UNLIKELY(this->size() >= this->capacity())) + if (LLVM_UNLIKELY(this->EndX >= this->CapacityX)) this->grow(); ::new ((void *)this->end()) T(std::forward(Args)...); - this->set_size(this->size() + 1); + this->setEnd(this->end() + 1); } SmallVectorImpl &operator=(const SmallVectorImpl &RHS); @@ -667,6 +673,20 @@ public: return std::lexicographical_compare(this->begin(), this->end(), RHS.begin(), RHS.end()); } + + /// Set the array size to \p N, which the current array must have enough + /// capacity for. + /// + /// This does not construct or destroy any elements in the vector. + /// + /// Clients can use this in conjunction with capacity() to write past the end + /// of the buffer when they know that more elements are available, and only + /// update the size later. This avoids the cost of value initializing elements + /// which will only be overwritten. + void set_size(size_type N) { + assert(N <= this->capacity()); + this->setEnd(this->begin() + N); + } }; template @@ -676,8 +696,8 @@ void SmallVectorImpl::swap(SmallVectorImpl &RHS) { // We can only avoid copying elements if neither vector is small. if (!this->isSmall() && !RHS.isSmall()) { std::swap(this->BeginX, RHS.BeginX); - std::swap(this->Size, RHS.Size); - std::swap(this->Capacity, RHS.Capacity); + std::swap(this->EndX, RHS.EndX); + std::swap(this->CapacityX, RHS.CapacityX); return; } if (RHS.size() > this->capacity()) @@ -695,15 +715,15 @@ void SmallVectorImpl::swap(SmallVectorImpl &RHS) { if (this->size() > RHS.size()) { size_t EltDiff = this->size() - RHS.size(); this->uninitialized_copy(this->begin()+NumShared, this->end(), RHS.end()); - RHS.set_size(RHS.size() + EltDiff); + RHS.setEnd(RHS.end()+EltDiff); this->destroy_range(this->begin()+NumShared, this->end()); - this->set_size(NumShared); + this->setEnd(this->begin()+NumShared); } else if (RHS.size() > this->size()) { size_t EltDiff = RHS.size() - this->size(); this->uninitialized_copy(RHS.begin()+NumShared, RHS.end(), this->end()); - this->set_size(this->size() + EltDiff); + this->setEnd(this->end() + EltDiff); this->destroy_range(RHS.begin()+NumShared, RHS.end()); - RHS.set_size(NumShared); + RHS.setEnd(RHS.begin()+NumShared); } } @@ -729,7 +749,7 @@ SmallVectorImpl &SmallVectorImpl:: this->destroy_range(NewEnd, this->end()); // Trim. - this->set_size(RHSSize); + this->setEnd(NewEnd); return *this; } @@ -739,7 +759,7 @@ SmallVectorImpl &SmallVectorImpl:: if (this->capacity() < RHSSize) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); - this->set_size(0); + this->setEnd(this->begin()); CurSize = 0; this->grow(RHSSize); } else if (CurSize) { @@ -752,7 +772,7 @@ SmallVectorImpl &SmallVectorImpl:: this->begin()+CurSize); // Set end. - this->set_size(RHSSize); + this->setEnd(this->begin()+RHSSize); return *this; } @@ -766,8 +786,8 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) { this->destroy_range(this->begin(), this->end()); if (!this->isSmall()) free(this->begin()); this->BeginX = RHS.BeginX; - this->Size = RHS.Size; - this->Capacity = RHS.Capacity; + this->EndX = RHS.EndX; + this->CapacityX = RHS.CapacityX; RHS.resetToSmall(); return *this; } @@ -784,7 +804,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) { // Destroy excess elements and trim the bounds. this->destroy_range(NewEnd, this->end()); - this->set_size(RHSSize); + this->setEnd(NewEnd); // Clear the RHS. RHS.clear(); @@ -799,7 +819,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) { if (this->capacity() < RHSSize) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); - this->set_size(0); + this->setEnd(this->begin()); CurSize = 0; this->grow(RHSSize); } else if (CurSize) { @@ -812,7 +832,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) { this->begin()+CurSize); // Set end. - this->set_size(RHSSize); + this->setEnd(this->begin()+RHSSize); RHS.clear(); return *this; diff --git a/lib/Support/SmallVector.cpp b/lib/Support/SmallVector.cpp index 7208572d08d..e8e3498968c 100644 --- a/lib/Support/SmallVector.cpp +++ b/lib/Support/SmallVector.cpp @@ -15,33 +15,30 @@ using namespace llvm; // Check that no bytes are wasted. -static_assert(sizeof(SmallVector) == - sizeof(unsigned) * 2 + sizeof(void *) * 2, +static_assert(sizeof(SmallVector) == sizeof(void *) * 4, "wasted space in SmallVector size 1; missing EBO?"); /// grow_pod - This is an implementation of the grow() method which only works /// on POD-like datatypes and is out of line to reduce code duplication. -void SmallVectorBase::grow_pod(void *FirstEl, size_t MinCapacity, +void SmallVectorBase::grow_pod(void *FirstEl, size_t MinSizeInBytes, size_t TSize) { - // Ensure we can fit the new capacity in 32 bits. - if (MinCapacity > UINT32_MAX) - report_bad_alloc_error("SmallVector capacity overflow during allocation"); - - size_t NewCapacity = 2 * capacity() + 1; // Always grow. - NewCapacity = - std::min(std::max(NewCapacity, MinCapacity), size_t(UINT32_MAX)); + size_t CurSizeBytes = size_in_bytes(); + size_t NewCapacityInBytes = 2 * capacity_in_bytes() + TSize; // Always grow. + if (NewCapacityInBytes < MinSizeInBytes) + NewCapacityInBytes = MinSizeInBytes; void *NewElts; if (BeginX == FirstEl) { - NewElts = safe_malloc(NewCapacity * TSize); + NewElts = safe_malloc(NewCapacityInBytes); // Copy the elements over. No need to run dtors on PODs. - memcpy(NewElts, this->BeginX, size() * TSize); + memcpy(NewElts, this->BeginX, CurSizeBytes); } else { // If this wasn't grown from the inline copy, grow the allocated space. - NewElts = safe_realloc(this->BeginX, NewCapacity * TSize); + NewElts = safe_realloc(this->BeginX, NewCapacityInBytes); } + this->EndX = (char*)NewElts+CurSizeBytes; this->BeginX = NewElts; - this->Capacity = NewCapacity; + this->CapacityX = (char*)this->BeginX + NewCapacityInBytes; }