[ADT] Rename NumNonEmpty to NumEntries in SmallPtrSet (NFC) (#153757)

Without this patch, we use NumNonEmpty, which keeps track of the
number of valid entries plus tombstones even though we have a separate
variable to keep track of the number of tombstones.

This patch simplifies the metadata.  Specifically, it changes the name
and semantics of the variable to NumEntries to keep track of the
number of valid entries.

The difference in semantics requires some code changes aside from
mechanical replacements:

- size() just returns NumEntries.

- erase_imp() and remove_if() need to decrement NumEntries in the
  large mode.

- insert_imp_big() increments NumEntries for successful insertions,
  regardless of whether a tombstone is being replaced with a valid
  entry.  It also computes the number of non-tombstone empty slots as:

  CurArraySize - NumEntries - NumTombstones

- Grow() no longer needs NumNonEmpty -= NumTombstones.

Overall, the resulting code should look more intuitive and more
consistent with DenseMapSet.

Author: kazutakahirata

llvm/include/llvm/ADT/SmallPtrSet.h

@@ -62,10 +62,10 @@ class SmallPtrSetImplBase : public DebugEpochBase {
   /// CurArraySize - The allocated size of CurArray, always a power of two.
   unsigned CurArraySize;
 
-  /// Number of elements in CurArray that contain a value or are a tombstone.
+  /// Number of elements in CurArray that contain a value.
   /// If small, all these elements are at the beginning of CurArray and the rest
   /// is uninitialized.
-  unsigned NumNonEmpty;
+  unsigned NumEntries;
   /// Number of tombstones in CurArray.
   unsigned NumTombstones;
   /// Whether the set is in small representation.
@@ -79,7 +79,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
                                SmallPtrSetImplBase &&that);
 
   explicit SmallPtrSetImplBase(const void **SmallStorage, unsigned SmallSize)
-      : CurArray(SmallStorage), CurArraySize(SmallSize), NumNonEmpty(0),
+      : CurArray(SmallStorage), CurArraySize(SmallSize), NumEntries(0),
         NumTombstones(0), IsSmall(true) {
     assert(llvm::has_single_bit(SmallSize) &&
            "Initial size must be a power of two!");
@@ -96,7 +96,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
   SmallPtrSetImplBase &operator=(const SmallPtrSetImplBase &) = delete;
 
   [[nodiscard]] bool empty() const { return size() == 0; }
-  size_type size() const { return NumNonEmpty - NumTombstones; }
+  size_type size() const { return NumEntries; }
   size_type capacity() const { return CurArraySize; }
 
   void clear() {
@@ -110,25 +110,25 @@ class SmallPtrSetImplBase : public DebugEpochBase {
       memset(CurArray, -1, CurArraySize * sizeof(void *));
     }
 
-    NumNonEmpty = 0;
+    NumEntries = 0;
     NumTombstones = 0;
   }
 
-  void reserve(size_type NumEntries) {
+  void reserve(size_type NewNumEntries) {
     incrementEpoch();
     // Do nothing if we're given zero as a reservation size.
-    if (NumEntries == 0)
+    if (NewNumEntries == 0)
       return;
-    // No need to expand if we're small and NumEntries will fit in the space.
-    if (isSmall() && NumEntries <= CurArraySize)
+    // No need to expand if we're small and NewNumEntries will fit in the space.
+    if (isSmall() && NewNumEntries <= CurArraySize)
       return;
     // insert_imp_big will reallocate if stores is more than 75% full, on the
     // /final/ insertion.
-    if (!isSmall() && ((NumEntries - 1) * 4) < (CurArraySize * 3))
+    if (!isSmall() && ((NewNumEntries - 1) * 4) < (CurArraySize * 3))
       return;
     // We must Grow -- find the size where we'd be 75% full, then round up to
     // the next power of two.
-    size_type NewSize = NumEntries + (NumEntries / 3);
+    size_type NewSize = NewNumEntries + (NewNumEntries / 3);
     NewSize = llvm::bit_ceil(NewSize);
     // Like insert_imp_big, always allocate at least 128 elements.
     NewSize = std::max(128u, NewSize);
@@ -145,15 +145,15 @@ class SmallPtrSetImplBase : public DebugEpochBase {
   }
 
   const void **EndPointer() const {
-    return isSmall() ? CurArray + NumNonEmpty : CurArray + CurArraySize;
+    return isSmall() ? CurArray + NumEntries : CurArray + CurArraySize;
   }
 
   iterator_range<const void **> small_buckets() {
-    return make_range(CurArray, CurArray + NumNonEmpty);
+    return make_range(CurArray, CurArray + NumEntries);
   }
 
   iterator_range<const void *const *> small_buckets() const {
-    return {CurArray, CurArray + NumNonEmpty};
+    return {CurArray, CurArray + NumEntries};
   }
 
   iterator_range<const void **> buckets() {
@@ -172,10 +172,10 @@ class SmallPtrSetImplBase : public DebugEpochBase {
       }
 
       // Nope, there isn't.  If we stay small, just 'pushback' now.
-      if (NumNonEmpty < CurArraySize) {
-        CurArray[NumNonEmpty++] = Ptr;
+      if (NumEntries < CurArraySize) {
+        CurArray[NumEntries++] = Ptr;
         incrementEpoch();
-        return std::make_pair(CurArray + (NumNonEmpty - 1), true);
+        return std::make_pair(CurArray + (NumEntries - 1), true);
       }
       // Otherwise, hit the big set case, which will call grow.
     }
@@ -190,7 +190,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
     if (isSmall()) {
       for (const void *&Bucket : small_buckets()) {
         if (Bucket == Ptr) {
-          Bucket = CurArray[--NumNonEmpty];
+          Bucket = CurArray[--NumEntries];
           incrementEpoch();
           return true;
         }
@@ -204,6 +204,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
 
     *const_cast<const void **>(Bucket) = getTombstoneMarker();
     NumTombstones++;
+    --NumEntries;
     // Treat this consistently from an API perspective, even if we don't
     // actually invalidate iterators here.
     incrementEpoch();
@@ -430,12 +431,12 @@ class SmallPtrSetImpl : public SmallPtrSetImplBase {
   bool remove_if(UnaryPredicate P) {
     bool Removed = false;
     if (isSmall()) {
-      const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
+      const void **APtr = CurArray, **E = CurArray + NumEntries;
       while (APtr != E) {
         PtrType Ptr = PtrTraits::getFromVoidPointer(const_cast<void *>(*APtr));
         if (P(Ptr)) {
           *APtr = *--E;
-          --NumNonEmpty;
+          --NumEntries;
           incrementEpoch();
           Removed = true;
         } else {
@@ -452,6 +453,7 @@ class SmallPtrSetImpl : public SmallPtrSetImplBase {
       if (P(Ptr)) {
         Bucket = getTombstoneMarker();
         ++NumTombstones;
+        --NumEntries;
         incrementEpoch();
         Removed = true;
       }

llvm/lib/Support/SmallPtrSet.cpp

@@ -28,7 +28,7 @@ void SmallPtrSetImplBase::shrink_and_clear() {
   // Reduce the number of buckets.
   unsigned Size = size();
   CurArraySize = Size > 16 ? 1 << (Log2_32_Ceil(Size) + 1) : 32;
-  NumNonEmpty = NumTombstones = 0;
+  NumEntries = NumTombstones = 0;
 
   // Install the new array.  Clear all the buckets to empty.
   CurArray = (const void**)safe_malloc(sizeof(void*) * CurArraySize);
@@ -41,7 +41,8 @@ SmallPtrSetImplBase::insert_imp_big(const void *Ptr) {
   if (LLVM_UNLIKELY(size() * 4 >= CurArraySize * 3)) {
     // If more than 3/4 of the array is full, grow.
     Grow(CurArraySize < 64 ? 128 : CurArraySize * 2);
-  } else if (LLVM_UNLIKELY(CurArraySize - NumNonEmpty < CurArraySize / 8)) {
+  } else if (LLVM_UNLIKELY(CurArraySize - NumEntries - NumTombstones <
+                           CurArraySize / 8)) {
     // If fewer of 1/8 of the array is empty (meaning that many are filled with
     // tombstones), rehash.
     Grow(CurArraySize);
@@ -55,8 +56,7 @@ SmallPtrSetImplBase::insert_imp_big(const void *Ptr) {
   // Otherwise, insert it!
   if (*Bucket == getTombstoneMarker())
     --NumTombstones;
-  else
-    ++NumNonEmpty; // Track density.
+  ++NumEntries;
   *Bucket = Ptr;
   incrementEpoch();
   return std::make_pair(Bucket, true);
@@ -130,7 +130,6 @@ void SmallPtrSetImplBase::Grow(unsigned NewSize) {
 
   if (!WasSmall)
     free(OldBuckets.begin());
-  NumNonEmpty -= NumTombstones;
   NumTombstones = 0;
   IsSmall = false;
 }
@@ -193,7 +192,7 @@ void SmallPtrSetImplBase::copyHelper(const SmallPtrSetImplBase &RHS) {
   // Copy over the contents from the other set
   std::copy(RHS.CurArray, RHS.EndPointer(), CurArray);
 
-  NumNonEmpty = RHS.NumNonEmpty;
+  NumEntries = RHS.NumEntries;
   NumTombstones = RHS.NumTombstones;
 }
 
@@ -215,21 +214,21 @@ void SmallPtrSetImplBase::moveHelper(const void **SmallStorage,
   if (RHS.isSmall()) {
     // Copy a small RHS rather than moving.
     CurArray = SmallStorage;
-    std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, CurArray);
+    std::copy(RHS.CurArray, RHS.CurArray + RHS.NumEntries, CurArray);
   } else {
     CurArray = RHS.CurArray;
     RHS.CurArray = RHSSmallStorage;
   }
 
   // Copy the rest of the trivial members.
   CurArraySize = RHS.CurArraySize;
-  NumNonEmpty = RHS.NumNonEmpty;
+  NumEntries = RHS.NumEntries;
   NumTombstones = RHS.NumTombstones;
   IsSmall = RHS.IsSmall;
 
   // Make the RHS small and empty.
   RHS.CurArraySize = SmallSize;
-  RHS.NumNonEmpty = 0;
+  RHS.NumEntries = 0;
   RHS.NumTombstones = 0;
   RHS.IsSmall = true;
 }
@@ -243,7 +242,7 @@ void SmallPtrSetImplBase::swap(const void **SmallStorage,
   if (!this->isSmall() && !RHS.isSmall()) {
     std::swap(this->CurArray, RHS.CurArray);
     std::swap(this->CurArraySize, RHS.CurArraySize);
-    std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
+    std::swap(this->NumEntries, RHS.NumEntries);
     std::swap(this->NumTombstones, RHS.NumTombstones);
     return;
   }
@@ -253,9 +252,9 @@ void SmallPtrSetImplBase::swap(const void **SmallStorage,
   // If only RHS is small, copy the small elements into LHS and move the pointer
   // from LHS to RHS.
   if (!this->isSmall() && RHS.isSmall()) {
-    std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, SmallStorage);
+    std::copy(RHS.CurArray, RHS.CurArray + RHS.NumEntries, SmallStorage);
     std::swap(RHS.CurArraySize, this->CurArraySize);
-    std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
+    std::swap(this->NumEntries, RHS.NumEntries);
     std::swap(this->NumTombstones, RHS.NumTombstones);
     RHS.CurArray = this->CurArray;
     RHS.IsSmall = false;
@@ -267,10 +266,10 @@ void SmallPtrSetImplBase::swap(const void **SmallStorage,
   // If only LHS is small, copy the small elements into RHS and move the pointer
   // from RHS to LHS.
   if (this->isSmall() && !RHS.isSmall()) {
-    std::copy(this->CurArray, this->CurArray + this->NumNonEmpty,
+    std::copy(this->CurArray, this->CurArray + this->NumEntries,
               RHSSmallStorage);
     std::swap(RHS.CurArraySize, this->CurArraySize);
-    std::swap(RHS.NumNonEmpty, this->NumNonEmpty);
+    std::swap(RHS.NumEntries, this->NumEntries);
     std::swap(RHS.NumTombstones, this->NumTombstones);
     this->CurArray = RHS.CurArray;
     this->IsSmall = false;
@@ -281,16 +280,16 @@ void SmallPtrSetImplBase::swap(const void **SmallStorage,
 
   // Both a small, just swap the small elements.
   assert(this->isSmall() && RHS.isSmall());
-  unsigned MinNonEmpty = std::min(this->NumNonEmpty, RHS.NumNonEmpty);
-  std::swap_ranges(this->CurArray, this->CurArray + MinNonEmpty, RHS.CurArray);
-  if (this->NumNonEmpty > MinNonEmpty) {
-    std::copy(this->CurArray + MinNonEmpty, this->CurArray + this->NumNonEmpty,
-              RHS.CurArray + MinNonEmpty);
+  unsigned MinEntries = std::min(this->NumEntries, RHS.NumEntries);
+  std::swap_ranges(this->CurArray, this->CurArray + MinEntries, RHS.CurArray);
+  if (this->NumEntries > MinEntries) {
+    std::copy(this->CurArray + MinEntries, this->CurArray + this->NumEntries,
+              RHS.CurArray + MinEntries);
   } else {
-    std::copy(RHS.CurArray + MinNonEmpty, RHS.CurArray + RHS.NumNonEmpty,
-              this->CurArray + MinNonEmpty);
+    std::copy(RHS.CurArray + MinEntries, RHS.CurArray + RHS.NumEntries,
+              this->CurArray + MinEntries);
   }
   assert(this->CurArraySize == RHS.CurArraySize);
-  std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
+  std::swap(this->NumEntries, RHS.NumEntries);
   std::swap(this->NumTombstones, RHS.NumTombstones);
 }