Revert "ADT: SmallVector size/capacity use word-size integers when elements are small"

This reverts commit b8d08e961df1d229872c785ebdbc8367432e9752.

This change causes a 1% compile-time and 1% memory usage regression:

http://llvm-compile-time-tracker.com/compare.php?from=73b7dd1fb3c17a4ac4b1f1e603f26fa708009649&to=b8d08e961df1d229872c785ebdbc8367432e9752&stat=instructions
http://llvm-compile-time-tracker.com/compare.php?from=73b7dd1fb3c17a4ac4b1f1e603f26fa708009649&to=b8d08e961df1d229872c785ebdbc8367432e9752&stat=max-rss

include/llvm/ADT/SmallVector.h

@@ -16,10 +16,10 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Compiler.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemAlloc.h"
 #include "llvm/Support/type_traits.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
@@ -27,7 +27,6 @@
 #include <cstring>
 #include <initializer_list>
 #include <iterator>
-#include <limits>
 #include <memory>
 #include <new>
 #include <type_traits>
@@ -35,23 +34,11 @@
 
 namespace llvm {
 
-/// This is all the stuff common to all SmallVectors.
-///
-/// The template parameter specifies the type which should be used to hold the
-/// Size and Capacity of the SmallVector, so it can be adjusted.
-/// Using 32 bit size is desirable to shink the size of the SmallVector.
-/// Using 64 bit size is desirable for cases like SmallVector<char>, where a
-/// 32 bit size would limit the vector to ~4GB. SmallVectors are used for
-/// buffering bitcode output - which can exceed 4GB.
-template <class Size_T> class SmallVectorBase {
+/// This is all the non-templated stuff common to all SmallVectors.
+class SmallVectorBase {
 protected:
   void *BeginX;
-  Size_T Size = 0, Capacity;
-
-  /// The maximum value of the Size_T used.
-  static constexpr size_t SizeTypeMax() {
-    return std::numeric_limits<Size_T>::max();
-  }
+  unsigned Size = 0, Capacity;
 
   SmallVectorBase() = delete;
   SmallVectorBase(void *FirstEl, size_t TotalCapacity)
@@ -59,39 +46,7 @@ protected:
 
   /// 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.
-  /// This function will report a fatal error if it cannot increase capacity.
-  void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize) {
-    // Ensure we can fit the new capacity.
-    // This is only going to be applicable if the when the capacity is 32 bit.
-    if (MinCapacity > SizeTypeMax())
-      report_bad_alloc_error("SmallVector capacity overflow during allocation");
-
-    // Ensure we can meet the guarantee of space for at least one more element.
-    // The above check alone will not catch the case where grow is called with a
-    // default MinCapacity of 0, but the current capacity cannot be increased.
-    // This is only going to be applicable if the when the capacity is 32 bit.
-    if (capacity() == SizeTypeMax())
-      report_bad_alloc_error("SmallVector capacity unable to grow");
-
-    // In theory 2*capacity can overflow if the capacity is 64 bit, but the
-    // original capacity would never be large enough for this to be a problem.
-    size_t NewCapacity = 2 * capacity() + 1; // Always grow.
-    NewCapacity = std::min(std::max(NewCapacity, MinCapacity), SizeTypeMax());
-
-    void *NewElts;
-    if (BeginX == FirstEl) {
-      NewElts = safe_malloc(NewCapacity * TSize);
-
-      // Copy the elements over.  No need to run dtors on PODs.
-      memcpy(NewElts, this->BeginX, size() * TSize);
-    } else {
-      // If this wasn't grown from the inline copy, grow the allocated space.
-      NewElts = safe_realloc(this->BeginX, NewCapacity * TSize);
-    }
-
-    this->BeginX = NewElts;
-    this->Capacity = NewCapacity;
-  }
+  void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize);
 
 public:
   size_t size() const { return Size; }
@@ -114,13 +69,9 @@ public:
   }
 };
 
-template <class T>
-using SmallVectorSizeType =
-    typename std::conditional<sizeof(T) < 4, uintptr_t, uint32_t>::type;
-
 /// Figure out the offset of the first element.
 template <class T, typename = void> struct SmallVectorAlignmentAndSize {
-  AlignedCharArrayUnion<SmallVectorBase<SmallVectorSizeType<T>>> Base;
+  AlignedCharArrayUnion<SmallVectorBase> Base;
   AlignedCharArrayUnion<T> FirstEl;
 };
 
@@ -128,10 +79,7 @@ template <class T, typename = void> struct SmallVectorAlignmentAndSize {
 /// the type T is a POD. The extra dummy template argument is used by ArrayRef
 /// to avoid unnecessarily requiring T to be complete.
 template <typename T, typename = void>
-class SmallVectorTemplateCommon
-    : public SmallVectorBase<SmallVectorSizeType<T>> {
-  using Base = SmallVectorBase<SmallVectorSizeType<T>>;
-
+class SmallVectorTemplateCommon : public SmallVectorBase {
   /// Find the address of the first element.  For this pointer math to be valid
   /// with small-size of 0 for T with lots of alignment, it's important that
   /// SmallVectorStorage is properly-aligned even for small-size of 0.
@@ -143,20 +91,21 @@ class SmallVectorTemplateCommon
   // Space after 'FirstEl' is clobbered, do not add any instance vars after it.
 
 protected:
-  SmallVectorTemplateCommon(size_t Size) : Base(getFirstEl(), Size) {}
+  SmallVectorTemplateCommon(size_t Size)
+      : SmallVectorBase(getFirstEl(), Size) {}
 
   void grow_pod(size_t MinCapacity, size_t TSize) {
-    Base::grow_pod(getFirstEl(), MinCapacity, TSize);
+    SmallVectorBase::grow_pod(getFirstEl(), MinCapacity, TSize);
   }
 
   /// Return true if this is a smallvector which has not had dynamic
   /// memory allocated for it.
-  bool isSmall() const { return this->BeginX == getFirstEl(); }
+  bool isSmall() const { return BeginX == getFirstEl(); }
 
   /// Put this vector in a state of being small.
   void resetToSmall() {
-    this->BeginX = getFirstEl();
-    this->Size = this->Capacity = 0; // FIXME: Setting Capacity to 0 is suspect.
+    BeginX = getFirstEl();
+    Size = Capacity = 0; // FIXME: Setting Capacity to 0 is suspect.
   }
 
 public:
@@ -174,10 +123,6 @@ public:
   using pointer = T *;
   using const_pointer = const T *;
 
-  using Base::capacity;
-  using Base::empty;
-  using Base::size;
-
   // forward iterator creation methods.
   iterator begin() { return (iterator)this->BeginX; }
   const_iterator begin() const { return (const_iterator)this->BeginX; }
@@ -191,9 +136,7 @@ public:
   const_reverse_iterator rend() const { return const_reverse_iterator(begin());}
 
   size_type size_in_bytes() const { return size() * sizeof(T); }
-  size_type max_size() const {
-    return std::min(this->SizeTypeMax(), size_type(-1) / sizeof(T));
-  }
+  size_type max_size() const { return size_type(-1) / sizeof(T); }
 
   size_t capacity_in_bytes() const { return capacity() * sizeof(T); }
 
@@ -288,21 +231,12 @@ public:
 // Define this out-of-line to dissuade the C++ compiler from inlining it.
 template <typename T, bool TriviallyCopyable>
 void SmallVectorTemplateBase<T, TriviallyCopyable>::grow(size_t MinSize) {
-  // Ensure we can fit the new capacity.
-  // This is only going to be applicable if the when the capacity is 32 bit.
-  if (MinSize > this->SizeTypeMax())
+  if (MinSize > UINT32_MAX)
     report_bad_alloc_error("SmallVector capacity overflow during allocation");
 
-  // Ensure we can meet the guarantee of space for at least one more element.
-  // The above check alone will not catch the case where grow is called with a
-  // default MinCapacity of 0, but the current capacity cannot be increased.
-  // This is only going to be applicable if the when the capacity is 32 bit.
-  if (this->capacity() == this->SizeTypeMax())
-    report_bad_alloc_error("SmallVector capacity unable to grow");
-
   // Always grow, even from zero.
   size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2));
-  NewCapacity = std::min(std::max(NewCapacity, MinSize), this->SizeTypeMax());
+  NewCapacity = std::min(std::max(NewCapacity, MinSize), size_t(UINT32_MAX));
   T *NewElts = static_cast<T*>(llvm::safe_malloc(NewCapacity*sizeof(T)));
 
   // Move the elements over.

lib/Support/SmallVector.cpp

@@ -36,6 +36,30 @@ static_assert(sizeof(SmallVector<Struct32B, 0>) >= alignof(Struct32B),
 static_assert(sizeof(SmallVector<void *, 1>) ==
                   sizeof(unsigned) * 2 + sizeof(void *) * 2,
               "wasted space in SmallVector size 1");
-static_assert(sizeof(SmallVector<char, 0>) ==
-                  sizeof(void *) * 2 + sizeof(void *),
-              "1 byte elements have word-sized type for size and capacity");
+
+/// 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,
+                               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));
+
+  void *NewElts;
+  if (BeginX == FirstEl) {
+    NewElts = safe_malloc(NewCapacity * TSize);
+
+    // Copy the elements over.  No need to run dtors on PODs.
+    memcpy(NewElts, this->BeginX, size() * TSize);
+  } else {
+    // If this wasn't grown from the inline copy, grow the allocated space.
+    NewElts = safe_realloc(this->BeginX, NewCapacity * TSize);
+  }
+
+  this->BeginX = NewElts;
+  this->Capacity = NewCapacity;
+}