[llvm] [SmallPtrSet] Remove SmallArray member (NFC) (PR #118099)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 29 07:24:53 PST 2024
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/118099
Currently SmallPtrSet stores CurArray and SmallArray, with the former pointing to the latter in small representation. Instead, only use CurArray and a separate IsSmall boolean.
Most of the implementation doesn't need the separate SmallArray member -- we only need it for copy/move/swap operations, where we may switch back from large to small representation. In those cases, we explicitly pass down the pointer to SmallStorage.
This reduces the size of SmallPtrSet and improves compile-time: https://llvm-compile-time-tracker.com/compare.php?from=352f8688d0ca250c9e8774321f6c3bcd4298cc09&to=f8b23930d54ecd06ec32e77aa90f4b774fe31841&stat=instructions:u
>From 51d73db20ed042cca1857eda1517d48dc919dd68 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Fri, 29 Nov 2024 12:20:00 +0100
Subject: [PATCH] [SmallPtrSet] Remove SmallArray member (NFC)
Currently SmallPtrSet stores CurArray and SmallArray, with the
former pointing to the latter in small representation. Instead,
only use CurArray and a separate IsSmall boolean.
Most of the implementation doesn't need the separate SmallArray
member -- we only need it for copy/move/swap operations, where we
may switch back from large to small representation. In those cases,
we explicitly pass down the pointer to SmallStorage.
---
llvm/include/llvm/ADT/SmallPtrSet.h | 53 +++++++++++----------
llvm/lib/Support/SmallPtrSet.cpp | 72 ++++++++++++++++-------------
2 files changed, 70 insertions(+), 55 deletions(-)
diff --git a/llvm/include/llvm/ADT/SmallPtrSet.h b/llvm/include/llvm/ADT/SmallPtrSet.h
index 1fc2318342ae78..1018114a307a4d 100644
--- a/llvm/include/llvm/ADT/SmallPtrSet.h
+++ b/llvm/include/llvm/ADT/SmallPtrSet.h
@@ -53,10 +53,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
friend class SmallPtrSetIteratorImpl;
protected:
- /// SmallArray - Points to a fixed size set of buckets, used in 'small mode'.
- const void **SmallArray;
- /// CurArray - This is the current set of buckets. If equal to SmallArray,
- /// then the set is in 'small mode'.
+ /// The current set of buckets, in either small or big representation.
const void **CurArray;
/// CurArraySize - The allocated size of CurArray, always a power of two.
unsigned CurArraySize;
@@ -67,16 +64,18 @@ class SmallPtrSetImplBase : public DebugEpochBase {
unsigned NumNonEmpty;
/// Number of tombstones in CurArray.
unsigned NumTombstones;
+ /// Whether the set is in small representation.
+ bool IsSmall;
// Helpers to copy and move construct a SmallPtrSet.
SmallPtrSetImplBase(const void **SmallStorage,
const SmallPtrSetImplBase &that);
SmallPtrSetImplBase(const void **SmallStorage, unsigned SmallSize,
- SmallPtrSetImplBase &&that);
+ const void **RHSSmallStorage, SmallPtrSetImplBase &&that);
explicit SmallPtrSetImplBase(const void **SmallStorage, unsigned SmallSize)
- : SmallArray(SmallStorage), CurArray(SmallStorage),
- CurArraySize(SmallSize), NumNonEmpty(0), NumTombstones(0) {
+ : CurArray(SmallStorage), CurArraySize(SmallSize), NumNonEmpty(0),
+ NumTombstones(0), IsSmall(true) {
assert(SmallSize && (SmallSize & (SmallSize-1)) == 0 &&
"Initial size must be a power of two!");
}
@@ -150,7 +149,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
std::pair<const void *const *, bool> insert_imp(const void *Ptr) {
if (isSmall()) {
// Check to see if it is already in the set.
- for (const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
+ for (const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
APtr != E; ++APtr) {
const void *Value = *APtr;
if (Value == Ptr)
@@ -159,9 +158,9 @@ class SmallPtrSetImplBase : public DebugEpochBase {
// Nope, there isn't. If we stay small, just 'pushback' now.
if (NumNonEmpty < CurArraySize) {
- SmallArray[NumNonEmpty++] = Ptr;
+ CurArray[NumNonEmpty++] = Ptr;
incrementEpoch();
- return std::make_pair(SmallArray + (NumNonEmpty - 1), true);
+ return std::make_pair(CurArray + (NumNonEmpty - 1), true);
}
// Otherwise, hit the big set case, which will call grow.
}
@@ -174,10 +173,10 @@ class SmallPtrSetImplBase : public DebugEpochBase {
/// in.
bool erase_imp(const void * Ptr) {
if (isSmall()) {
- for (const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
+ for (const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
APtr != E; ++APtr) {
if (*APtr == Ptr) {
- *APtr = SmallArray[--NumNonEmpty];
+ *APtr = CurArray[--NumNonEmpty];
incrementEpoch();
return true;
}
@@ -203,8 +202,9 @@ class SmallPtrSetImplBase : public DebugEpochBase {
const void *const * find_imp(const void * Ptr) const {
if (isSmall()) {
// Linear search for the item.
- for (const void *const *APtr = SmallArray,
- *const *E = SmallArray + NumNonEmpty; APtr != E; ++APtr)
+ for (const void *const *APtr = CurArray, *const *E =
+ CurArray + NumNonEmpty;
+ APtr != E; ++APtr)
if (*APtr == Ptr)
return APtr;
return EndPointer();
@@ -216,7 +216,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
return EndPointer();
}
- bool isSmall() const { return CurArray == SmallArray; }
+ bool isSmall() const { return IsSmall; }
private:
std::pair<const void *const *, bool> insert_imp_big(const void *Ptr);
@@ -231,14 +231,17 @@ class SmallPtrSetImplBase : public DebugEpochBase {
protected:
/// swap - Swaps the elements of two sets.
/// Note: This method assumes that both sets have the same small size.
- void swap(SmallPtrSetImplBase &RHS);
+ void swap(const void **SmallStorage, const void **RHSSmallStorage,
+ SmallPtrSetImplBase &RHS);
- void CopyFrom(const SmallPtrSetImplBase &RHS);
- void MoveFrom(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
+ void CopyFrom(const void **SmallStorage, const SmallPtrSetImplBase &RHS);
+ void MoveFrom(const void **SmallStorage, unsigned SmallSize,
+ const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
private:
/// Code shared by MoveFrom() and move constructor.
- void MoveHelper(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
+ void MoveHelper(const void **SmallStorage, unsigned SmallSize,
+ const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
/// Code shared by CopyFrom() and copy constructor.
void CopyHelper(const SmallPtrSetImplBase &RHS);
};
@@ -401,7 +404,7 @@ class SmallPtrSetImpl : public SmallPtrSetImplBase {
bool remove_if(UnaryPredicate P) {
bool Removed = false;
if (isSmall()) {
- const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
+ const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
while (APtr != E) {
PtrType Ptr = PtrTraits::getFromVoidPointer(const_cast<void *>(*APtr));
if (P(Ptr)) {
@@ -526,7 +529,8 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
SmallPtrSet() : BaseT(SmallStorage, SmallSizePowTwo) {}
SmallPtrSet(const SmallPtrSet &that) : BaseT(SmallStorage, that) {}
SmallPtrSet(SmallPtrSet &&that)
- : BaseT(SmallStorage, SmallSizePowTwo, std::move(that)) {}
+ : BaseT(SmallStorage, SmallSizePowTwo, that.SmallStorage,
+ std::move(that)) {}
template<typename It>
SmallPtrSet(It I, It E) : BaseT(SmallStorage, SmallSizePowTwo) {
@@ -541,14 +545,15 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
SmallPtrSet<PtrType, SmallSize> &
operator=(const SmallPtrSet<PtrType, SmallSize> &RHS) {
if (&RHS != this)
- this->CopyFrom(RHS);
+ this->CopyFrom(SmallStorage, RHS);
return *this;
}
SmallPtrSet<PtrType, SmallSize> &
operator=(SmallPtrSet<PtrType, SmallSize> &&RHS) {
if (&RHS != this)
- this->MoveFrom(SmallSizePowTwo, std::move(RHS));
+ this->MoveFrom(SmallStorage, SmallSizePowTwo, RHS.SmallStorage,
+ std::move(RHS));
return *this;
}
@@ -561,7 +566,7 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
/// swap - Swaps the elements of two sets.
void swap(SmallPtrSet<PtrType, SmallSize> &RHS) {
- SmallPtrSetImplBase::swap(RHS);
+ SmallPtrSetImplBase::swap(SmallStorage, RHS.SmallStorage, RHS);
}
};
diff --git a/llvm/lib/Support/SmallPtrSet.cpp b/llvm/lib/Support/SmallPtrSet.cpp
index 1bec911f39b13e..74662f3a3461db 100644
--- a/llvm/lib/Support/SmallPtrSet.cpp
+++ b/llvm/lib/Support/SmallPtrSet.cpp
@@ -134,17 +134,17 @@ void SmallPtrSetImplBase::Grow(unsigned NewSize) {
free(OldBuckets);
NumNonEmpty -= NumTombstones;
NumTombstones = 0;
+ IsSmall = false;
}
SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
const SmallPtrSetImplBase &that) {
- SmallArray = SmallStorage;
-
- // If we're becoming small, prepare to insert into our stack space
- if (that.isSmall()) {
- CurArray = SmallArray;
- // Otherwise, allocate new heap space (unless we were the same size)
+ IsSmall = that.isSmall();
+ if (IsSmall) {
+ // If we're becoming small, prepare to insert into our stack space
+ CurArray = SmallStorage;
} else {
+ // Otherwise, allocate new heap space (unless we were the same size)
CurArray = (const void**)safe_malloc(sizeof(void*) * that.CurArraySize);
}
@@ -154,12 +154,13 @@ SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
unsigned SmallSize,
+ const void **RHSSmallStorage,
SmallPtrSetImplBase &&that) {
- SmallArray = SmallStorage;
- MoveHelper(SmallSize, std::move(that));
+ MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(that));
}
-void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
+void SmallPtrSetImplBase::CopyFrom(const void **SmallStorage,
+ const SmallPtrSetImplBase &RHS) {
assert(&RHS != this && "Self-copy should be handled by the caller.");
if (isSmall() && RHS.isSmall())
@@ -170,8 +171,9 @@ void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
if (RHS.isSmall()) {
if (!isSmall())
free(CurArray);
- CurArray = SmallArray;
- // Otherwise, allocate new heap space (unless we were the same size)
+ CurArray = SmallStorage;
+ IsSmall = true;
+ // Otherwise, allocate new heap space (unless we were the same size)
} else if (CurArraySize != RHS.CurArraySize) {
if (isSmall())
CurArray = (const void**)safe_malloc(sizeof(void*) * RHS.CurArraySize);
@@ -180,6 +182,7 @@ void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
sizeof(void*) * RHS.CurArraySize);
CurArray = T;
}
+ IsSmall = false;
}
CopyHelper(RHS);
@@ -196,39 +199,46 @@ void SmallPtrSetImplBase::CopyHelper(const SmallPtrSetImplBase &RHS) {
NumTombstones = RHS.NumTombstones;
}
-void SmallPtrSetImplBase::MoveFrom(unsigned SmallSize,
+void SmallPtrSetImplBase::MoveFrom(const void **SmallStorage,
+ unsigned SmallSize,
+ const void **RHSSmallStorage,
SmallPtrSetImplBase &&RHS) {
if (!isSmall())
free(CurArray);
- MoveHelper(SmallSize, std::move(RHS));
+ MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(RHS));
}
-void SmallPtrSetImplBase::MoveHelper(unsigned SmallSize,
+void SmallPtrSetImplBase::MoveHelper(const void **SmallStorage,
+ unsigned SmallSize,
+ const void **RHSSmallStorage,
SmallPtrSetImplBase &&RHS) {
assert(&RHS != this && "Self-move should be handled by the caller.");
if (RHS.isSmall()) {
// Copy a small RHS rather than moving.
- CurArray = SmallArray;
+ CurArray = SmallStorage;
std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, CurArray);
} else {
CurArray = RHS.CurArray;
- RHS.CurArray = RHS.SmallArray;
+ RHS.CurArray = RHSSmallStorage;
}
// Copy the rest of the trivial members.
CurArraySize = RHS.CurArraySize;
NumNonEmpty = RHS.NumNonEmpty;
NumTombstones = RHS.NumTombstones;
+ IsSmall = RHS.IsSmall;
// Make the RHS small and empty.
RHS.CurArraySize = SmallSize;
- assert(RHS.CurArray == RHS.SmallArray);
RHS.NumNonEmpty = 0;
RHS.NumTombstones = 0;
+ RHS.IsSmall = true;
}
-void SmallPtrSetImplBase::swap(SmallPtrSetImplBase &RHS) {
+void SmallPtrSetImplBase::swap(const void **SmallStorage,
+ const void **RHSSmallStorage,
+ SmallPtrSetImplBase &RHS) {
if (this == &RHS) return;
// We can only avoid copying elements if neither set is small.
@@ -245,42 +255,42 @@ void SmallPtrSetImplBase::swap(SmallPtrSetImplBase &RHS) {
// If only RHS is small, copy the small elements into LHS and move the pointer
// from LHS to RHS.
if (!this->isSmall() && RHS.isSmall()) {
- assert(RHS.CurArray == RHS.SmallArray);
- std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, this->SmallArray);
+ std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, SmallStorage);
std::swap(RHS.CurArraySize, this->CurArraySize);
std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
std::swap(this->NumTombstones, RHS.NumTombstones);
RHS.CurArray = this->CurArray;
- this->CurArray = this->SmallArray;
+ RHS.IsSmall = false;
+ this->CurArray = SmallStorage;
+ this->IsSmall = true;
return;
}
// If only LHS is small, copy the small elements into RHS and move the pointer
// from RHS to LHS.
if (this->isSmall() && !RHS.isSmall()) {
- assert(this->CurArray == this->SmallArray);
std::copy(this->CurArray, this->CurArray + this->NumNonEmpty,
- RHS.SmallArray);
+ RHSSmallStorage);
std::swap(RHS.CurArraySize, this->CurArraySize);
std::swap(RHS.NumNonEmpty, this->NumNonEmpty);
std::swap(RHS.NumTombstones, this->NumTombstones);
this->CurArray = RHS.CurArray;
- RHS.CurArray = RHS.SmallArray;
+ this->IsSmall = false;
+ RHS.CurArray = RHSSmallStorage;
+ RHS.IsSmall = true;
return;
}
// 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->SmallArray, this->SmallArray + MinNonEmpty,
- RHS.SmallArray);
+ std::swap_ranges(this->CurArray, this->CurArray + MinNonEmpty, RHS.CurArray);
if (this->NumNonEmpty > MinNonEmpty) {
- std::copy(this->SmallArray + MinNonEmpty,
- this->SmallArray + this->NumNonEmpty,
- RHS.SmallArray + MinNonEmpty);
+ std::copy(this->CurArray + MinNonEmpty, this->CurArray + this->NumNonEmpty,
+ RHS.CurArray + MinNonEmpty);
} else {
- std::copy(RHS.SmallArray + MinNonEmpty, RHS.SmallArray + RHS.NumNonEmpty,
- this->SmallArray + MinNonEmpty);
+ std::copy(RHS.CurArray + MinNonEmpty, RHS.CurArray + RHS.NumNonEmpty,
+ this->CurArray + MinNonEmpty);
}
assert(this->CurArraySize == RHS.CurArraySize);
std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
More information about the llvm-commits
mailing list