[llvm] [SmallPtrSet] Remove SmallArray member (NFC) (PR #118099)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 29 13:00:20 PST 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/118099
>From 8fefed4166135a648aae20da47be413725164741 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 1/2] [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 | 57 ++++++++++++-----------
llvm/lib/Support/SmallPtrSet.cpp | 72 ++++++++++++++++-------------
2 files changed, 72 insertions(+), 57 deletions(-)
diff --git a/llvm/include/llvm/ADT/SmallPtrSet.h b/llvm/include/llvm/ADT/SmallPtrSet.h
index b15845f3b76a7e..1f68129da2e9d3 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();
@@ -219,8 +219,8 @@ class SmallPtrSetImplBase : public DebugEpochBase {
bool contains_imp(const void *Ptr) const {
if (isSmall()) {
// Linear search for the item.
- const void *const *APtr = SmallArray;
- const void *const *E = SmallArray + NumNonEmpty;
+ const void *const *APtr = CurArray;
+ const void *const *E = CurArray + NumNonEmpty;
for (; APtr != E; ++APtr)
if (*APtr == Ptr)
return true;
@@ -230,7 +230,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
return doFind(Ptr) != nullptr;
}
- bool isSmall() const { return CurArray == SmallArray; }
+ bool isSmall() const { return IsSmall; }
private:
std::pair<const void *const *, bool> insert_imp_big(const void *Ptr);
@@ -245,14 +245,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);
};
@@ -415,7 +418,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)) {
@@ -540,7 +543,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) {
@@ -555,14 +559,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;
}
@@ -575,7 +580,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);
>From 9bed44e14cb47c0ecf65d9b0d3455384ad58a104 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv at gmail.com>
Date: Fri, 29 Nov 2024 21:56:54 +0100
Subject: [PATCH 2/2] rename methods to comply with modern style
---
llvm/include/llvm/ADT/SmallPtrSet.h | 16 ++++++++--------
llvm/lib/Support/SmallPtrSet.cpp | 16 ++++++++--------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/ADT/SmallPtrSet.h b/llvm/include/llvm/ADT/SmallPtrSet.h
index 1f68129da2e9d3..cf6abc9129b40e 100644
--- a/llvm/include/llvm/ADT/SmallPtrSet.h
+++ b/llvm/include/llvm/ADT/SmallPtrSet.h
@@ -248,16 +248,16 @@ class SmallPtrSetImplBase : public DebugEpochBase {
void swap(const void **SmallStorage, const void **RHSSmallStorage,
SmallPtrSetImplBase &RHS);
- void CopyFrom(const void **SmallStorage, const SmallPtrSetImplBase &RHS);
- void MoveFrom(const void **SmallStorage, unsigned SmallSize,
+ 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(const void **SmallStorage, unsigned SmallSize,
+ /// Code shared by moveFrom() and move constructor.
+ void moveHelper(const void **SmallStorage, unsigned SmallSize,
const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
- /// Code shared by CopyFrom() and copy constructor.
- void CopyHelper(const SmallPtrSetImplBase &RHS);
+ /// Code shared by copyFrom() and copy constructor.
+ void copyHelper(const SmallPtrSetImplBase &RHS);
};
/// SmallPtrSetIteratorImpl - This is the common base class shared between all
@@ -559,14 +559,14 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
SmallPtrSet<PtrType, SmallSize> &
operator=(const SmallPtrSet<PtrType, SmallSize> &RHS) {
if (&RHS != this)
- this->CopyFrom(SmallStorage, RHS);
+ this->copyFrom(SmallStorage, RHS);
return *this;
}
SmallPtrSet<PtrType, SmallSize> &
operator=(SmallPtrSet<PtrType, SmallSize> &&RHS) {
if (&RHS != this)
- this->MoveFrom(SmallStorage, SmallSizePowTwo, RHS.SmallStorage,
+ this->moveFrom(SmallStorage, SmallSizePowTwo, RHS.SmallStorage,
std::move(RHS));
return *this;
}
diff --git a/llvm/lib/Support/SmallPtrSet.cpp b/llvm/lib/Support/SmallPtrSet.cpp
index 74662f3a3461db..83143a7fe80fad 100644
--- a/llvm/lib/Support/SmallPtrSet.cpp
+++ b/llvm/lib/Support/SmallPtrSet.cpp
@@ -149,17 +149,17 @@ SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
}
// Copy over the that array.
- CopyHelper(that);
+ copyHelper(that);
}
SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
unsigned SmallSize,
const void **RHSSmallStorage,
SmallPtrSetImplBase &&that) {
- MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(that));
+ moveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(that));
}
-void SmallPtrSetImplBase::CopyFrom(const void **SmallStorage,
+void SmallPtrSetImplBase::copyFrom(const void **SmallStorage,
const SmallPtrSetImplBase &RHS) {
assert(&RHS != this && "Self-copy should be handled by the caller.");
@@ -185,10 +185,10 @@ void SmallPtrSetImplBase::CopyFrom(const void **SmallStorage,
IsSmall = false;
}
- CopyHelper(RHS);
+ copyHelper(RHS);
}
-void SmallPtrSetImplBase::CopyHelper(const SmallPtrSetImplBase &RHS) {
+void SmallPtrSetImplBase::copyHelper(const SmallPtrSetImplBase &RHS) {
// Copy over the new array size
CurArraySize = RHS.CurArraySize;
@@ -199,16 +199,16 @@ void SmallPtrSetImplBase::CopyHelper(const SmallPtrSetImplBase &RHS) {
NumTombstones = RHS.NumTombstones;
}
-void SmallPtrSetImplBase::MoveFrom(const void **SmallStorage,
+void SmallPtrSetImplBase::moveFrom(const void **SmallStorage,
unsigned SmallSize,
const void **RHSSmallStorage,
SmallPtrSetImplBase &&RHS) {
if (!isSmall())
free(CurArray);
- MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(RHS));
+ moveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(RHS));
}
-void SmallPtrSetImplBase::MoveHelper(const void **SmallStorage,
+void SmallPtrSetImplBase::moveHelper(const void **SmallStorage,
unsigned SmallSize,
const void **RHSSmallStorage,
SmallPtrSetImplBase &&RHS) {
More information about the llvm-commits
mailing list