Skip to content

Commit 51d73db

Browse files
nikicnikic
committed
[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.
1 parent 352f868 commit 51d73db

File tree

2 files changed

+70
-55
lines changed

2 files changed

+70
-55
lines changed

‎llvm/include/llvm/ADT/SmallPtrSet.h‎

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
5353
friend class SmallPtrSetIteratorImpl;
5454

5555
protected:
56-
/// SmallArray - Points to a fixed size set of buckets, used in 'small mode'.
57-
const void **SmallArray;
58-
/// CurArray - This is the current set of buckets. If equal to SmallArray,
59-
/// then the set is in 'small mode'.
56+
/// The current set of buckets, in either small or big representation.
6057
const void **CurArray;
6158
/// CurArraySize - The allocated size of CurArray, always a power of two.
6259
unsigned CurArraySize;
@@ -67,16 +64,18 @@ class SmallPtrSetImplBase : public DebugEpochBase {
6764
unsigned NumNonEmpty;
6865
/// Number of tombstones in CurArray.
6966
unsigned NumTombstones;
67+
/// Whether the set is in small representation.
68+
bool IsSmall;
7069

7170
// Helpers to copy and move construct a SmallPtrSet.
7271
SmallPtrSetImplBase(const void **SmallStorage,
7372
const SmallPtrSetImplBase &that);
7473
SmallPtrSetImplBase(const void **SmallStorage, unsigned SmallSize,
75-
SmallPtrSetImplBase &&that);
74+
const void **RHSSmallStorage, SmallPtrSetImplBase &&that);
7675

7776
explicit SmallPtrSetImplBase(const void **SmallStorage, unsigned SmallSize)
78-
: SmallArray(SmallStorage), CurArray(SmallStorage),
79-
CurArraySize(SmallSize), NumNonEmpty(0), NumTombstones(0) {
77+
: CurArray(SmallStorage), CurArraySize(SmallSize), NumNonEmpty(0),
78+
NumTombstones(0), IsSmall(true) {
8079
assert(SmallSize && (SmallSize & (SmallSize-1)) == 0 &&
8180
"Initial size must be a power of two!");
8281
}
@@ -150,7 +149,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
150149
std::pair<const void *const *, bool> insert_imp(const void *Ptr) {
151150
if (isSmall()) {
152151
// Check to see if it is already in the set.
153-
for (const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
152+
for (const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
154153
APtr != E; ++APtr) {
155154
const void *Value = *APtr;
156155
if (Value == Ptr)
@@ -159,9 +158,9 @@ class SmallPtrSetImplBase : public DebugEpochBase {
159158

160159
// Nope, there isn't. If we stay small, just 'pushback' now.
161160
if (NumNonEmpty < CurArraySize) {
162-
SmallArray[NumNonEmpty++] = Ptr;
161+
CurArray[NumNonEmpty++] = Ptr;
163162
incrementEpoch();
164-
return std::make_pair(SmallArray + (NumNonEmpty - 1), true);
163+
return std::make_pair(CurArray + (NumNonEmpty - 1), true);
165164
}
166165
// Otherwise, hit the big set case, which will call grow.
167166
}
@@ -174,10 +173,10 @@ class SmallPtrSetImplBase : public DebugEpochBase {
174173
/// in.
175174
bool erase_imp(const void * Ptr) {
176175
if (isSmall()) {
177-
for (const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
176+
for (const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
178177
APtr != E; ++APtr) {
179178
if (*APtr == Ptr) {
180-
*APtr = SmallArray[--NumNonEmpty];
179+
*APtr = CurArray[--NumNonEmpty];
181180
incrementEpoch();
182181
return true;
183182
}
@@ -203,8 +202,9 @@ class SmallPtrSetImplBase : public DebugEpochBase {
203202
const void *const * find_imp(const void * Ptr) const {
204203
if (isSmall()) {
205204
// Linear search for the item.
206-
for (const void *const *APtr = SmallArray,
207-
*const *E = SmallArray + NumNonEmpty; APtr != E; ++APtr)
205+
for (const void *const *APtr = CurArray, *const *E =
206+
CurArray + NumNonEmpty;
207+
APtr != E; ++APtr)
208208
if (*APtr == Ptr)
209209
return APtr;
210210
return EndPointer();
@@ -216,7 +216,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
216216
return EndPointer();
217217
}
218218

219-
bool isSmall() const { return CurArray == SmallArray; }
219+
bool isSmall() const { return IsSmall; }
220220

221221
private:
222222
std::pair<const void *const *, bool> insert_imp_big(const void *Ptr);
@@ -231,14 +231,17 @@ class SmallPtrSetImplBase : public DebugEpochBase {
231231
protected:
232232
/// swap - Swaps the elements of two sets.
233233
/// Note: This method assumes that both sets have the same small size.
234-
void swap(SmallPtrSetImplBase &RHS);
234+
void swap(const void **SmallStorage, const void **RHSSmallStorage,
235+
SmallPtrSetImplBase &RHS);
235236

236-
void CopyFrom(const SmallPtrSetImplBase &RHS);
237-
void MoveFrom(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
237+
void CopyFrom(const void **SmallStorage, const SmallPtrSetImplBase &RHS);
238+
void MoveFrom(const void **SmallStorage, unsigned SmallSize,
239+
const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
238240

239241
private:
240242
/// Code shared by MoveFrom() and move constructor.
241-
void MoveHelper(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
243+
void MoveHelper(const void **SmallStorage, unsigned SmallSize,
244+
const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
242245
/// Code shared by CopyFrom() and copy constructor.
243246
void CopyHelper(const SmallPtrSetImplBase &RHS);
244247
};
@@ -401,7 +404,7 @@ class SmallPtrSetImpl : public SmallPtrSetImplBase {
401404
bool remove_if(UnaryPredicate P) {
402405
bool Removed = false;
403406
if (isSmall()) {
404-
const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
407+
const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
405408
while (APtr != E) {
406409
PtrType Ptr = PtrTraits::getFromVoidPointer(const_cast<void *>(*APtr));
407410
if (P(Ptr)) {
@@ -526,7 +529,8 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
526529
SmallPtrSet() : BaseT(SmallStorage, SmallSizePowTwo) {}
527530
SmallPtrSet(const SmallPtrSet &that) : BaseT(SmallStorage, that) {}
528531
SmallPtrSet(SmallPtrSet &&that)
529-
: BaseT(SmallStorage, SmallSizePowTwo, std::move(that)) {}
532+
: BaseT(SmallStorage, SmallSizePowTwo, that.SmallStorage,
533+
std::move(that)) {}
530534

531535
template<typename It>
532536
SmallPtrSet(It I, It E) : BaseT(SmallStorage, SmallSizePowTwo) {
@@ -541,14 +545,15 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
541545
SmallPtrSet<PtrType, SmallSize> &
542546
operator=(const SmallPtrSet<PtrType, SmallSize> &RHS) {
543547
if (&RHS != this)
544-
this->CopyFrom(RHS);
548+
this->CopyFrom(SmallStorage, RHS);
545549
return *this;
546550
}
547551

548552
SmallPtrSet<PtrType, SmallSize> &
549553
operator=(SmallPtrSet<PtrType, SmallSize> &&RHS) {
550554
if (&RHS != this)
551-
this->MoveFrom(SmallSizePowTwo, std::move(RHS));
555+
this->MoveFrom(SmallStorage, SmallSizePowTwo, RHS.SmallStorage,
556+
std::move(RHS));
552557
return *this;
553558
}
554559

@@ -561,7 +566,7 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
561566

562567
/// swap - Swaps the elements of two sets.
563568
void swap(SmallPtrSet<PtrType, SmallSize> &RHS) {
564-
SmallPtrSetImplBase::swap(RHS);
569+
SmallPtrSetImplBase::swap(SmallStorage, RHS.SmallStorage, RHS);
565570
}
566571
};
567572

‎llvm/lib/Support/SmallPtrSet.cpp‎

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -134,17 +134,17 @@ void SmallPtrSetImplBase::Grow(unsigned NewSize) {
134134
free(OldBuckets);
135135
NumNonEmpty -= NumTombstones;
136136
NumTombstones = 0;
137+
IsSmall = false;
137138
}
138139

139140
SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
140141
const SmallPtrSetImplBase &that) {
141-
SmallArray = SmallStorage;
142-
143-
// If we're becoming small, prepare to insert into our stack space
144-
if (that.isSmall()) {
145-
CurArray = SmallArray;
146-
// Otherwise, allocate new heap space (unless we were the same size)
142+
IsSmall = that.isSmall();
143+
if (IsSmall) {
144+
// If we're becoming small, prepare to insert into our stack space
145+
CurArray = SmallStorage;
147146
} else {
147+
// Otherwise, allocate new heap space (unless we were the same size)
148148
CurArray = (const void**)safe_malloc(sizeof(void*) * that.CurArraySize);
149149
}
150150

@@ -154,12 +154,13 @@ SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
154154

155155
SmallPtrSetImplBase::SmallPtrSetImplBase(const void **SmallStorage,
156156
unsigned SmallSize,
157+
const void **RHSSmallStorage,
157158
SmallPtrSetImplBase &&that) {
158-
SmallArray = SmallStorage;
159-
MoveHelper(SmallSize, std::move(that));
159+
MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(that));
160160
}
161161

162-
void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
162+
void SmallPtrSetImplBase::CopyFrom(const void **SmallStorage,
163+
const SmallPtrSetImplBase &RHS) {
163164
assert(&RHS != this && "Self-copy should be handled by the caller.");
164165

165166
if (isSmall() && RHS.isSmall())
@@ -170,8 +171,9 @@ void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
170171
if (RHS.isSmall()) {
171172
if (!isSmall())
172173
free(CurArray);
173-
CurArray = SmallArray;
174-
// Otherwise, allocate new heap space (unless we were the same size)
174+
CurArray = SmallStorage;
175+
IsSmall = true;
176+
// Otherwise, allocate new heap space (unless we were the same size)
175177
} else if (CurArraySize != RHS.CurArraySize) {
176178
if (isSmall())
177179
CurArray = (const void**)safe_malloc(sizeof(void*) * RHS.CurArraySize);
@@ -180,6 +182,7 @@ void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
180182
sizeof(void*) * RHS.CurArraySize);
181183
CurArray = T;
182184
}
185+
IsSmall = false;
183186
}
184187

185188
CopyHelper(RHS);
@@ -196,39 +199,46 @@ void SmallPtrSetImplBase::CopyHelper(const SmallPtrSetImplBase &RHS) {
196199
NumTombstones = RHS.NumTombstones;
197200
}
198201

199-
void SmallPtrSetImplBase::MoveFrom(unsigned SmallSize,
202+
void SmallPtrSetImplBase::MoveFrom(const void **SmallStorage,
203+
unsigned SmallSize,
204+
const void **RHSSmallStorage,
200205
SmallPtrSetImplBase &&RHS) {
201206
if (!isSmall())
202207
free(CurArray);
203-
MoveHelper(SmallSize, std::move(RHS));
208+
MoveHelper(SmallStorage, SmallSize, RHSSmallStorage, std::move(RHS));
204209
}
205210

206-
void SmallPtrSetImplBase::MoveHelper(unsigned SmallSize,
211+
void SmallPtrSetImplBase::MoveHelper(const void **SmallStorage,
212+
unsigned SmallSize,
213+
const void **RHSSmallStorage,
207214
SmallPtrSetImplBase &&RHS) {
208215
assert(&RHS != this && "Self-move should be handled by the caller.");
209216

210217
if (RHS.isSmall()) {
211218
// Copy a small RHS rather than moving.
212-
CurArray = SmallArray;
219+
CurArray = SmallStorage;
213220
std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, CurArray);
214221
} else {
215222
CurArray = RHS.CurArray;
216-
RHS.CurArray = RHS.SmallArray;
223+
RHS.CurArray = RHSSmallStorage;
217224
}
218225

219226
// Copy the rest of the trivial members.
220227
CurArraySize = RHS.CurArraySize;
221228
NumNonEmpty = RHS.NumNonEmpty;
222229
NumTombstones = RHS.NumTombstones;
230+
IsSmall = RHS.IsSmall;
223231

224232
// Make the RHS small and empty.
225233
RHS.CurArraySize = SmallSize;
226-
assert(RHS.CurArray == RHS.SmallArray);
227234
RHS.NumNonEmpty = 0;
228235
RHS.NumTombstones = 0;
236+
RHS.IsSmall = true;
229237
}
230238

231-
void SmallPtrSetImplBase::swap(SmallPtrSetImplBase &RHS) {
239+
void SmallPtrSetImplBase::swap(const void **SmallStorage,
240+
const void **RHSSmallStorage,
241+
SmallPtrSetImplBase &RHS) {
232242
if (this == &RHS) return;
233243

234244
// We can only avoid copying elements if neither set is small.
@@ -245,42 +255,42 @@ void SmallPtrSetImplBase::swap(SmallPtrSetImplBase &RHS) {
245255
// If only RHS is small, copy the small elements into LHS and move the pointer
246256
// from LHS to RHS.
247257
if (!this->isSmall() && RHS.isSmall()) {
248-
assert(RHS.CurArray == RHS.SmallArray);
249-
std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, this->SmallArray);
258+
std::copy(RHS.CurArray, RHS.CurArray + RHS.NumNonEmpty, SmallStorage);
250259
std::swap(RHS.CurArraySize, this->CurArraySize);
251260
std::swap(this->NumNonEmpty, RHS.NumNonEmpty);
252261
std::swap(this->NumTombstones, RHS.NumTombstones);
253262
RHS.CurArray = this->CurArray;
254-
this->CurArray = this->SmallArray;
263+
RHS.IsSmall = false;
264+
this->CurArray = SmallStorage;
265+
this->IsSmall = true;
255266
return;
256267
}
257268

258269
// If only LHS is small, copy the small elements into RHS and move the pointer
259270
// from RHS to LHS.
260271
if (this->isSmall() && !RHS.isSmall()) {
261-
assert(this->CurArray == this->SmallArray);
262272
std::copy(this->CurArray, this->CurArray + this->NumNonEmpty,
263-
RHS.SmallArray);
273+
RHSSmallStorage);
264274
std::swap(RHS.CurArraySize, this->CurArraySize);
265275
std::swap(RHS.NumNonEmpty, this->NumNonEmpty);
266276
std::swap(RHS.NumTombstones, this->NumTombstones);
267277
this->CurArray = RHS.CurArray;
268-
RHS.CurArray = RHS.SmallArray;
278+
this->IsSmall = false;
279+
RHS.CurArray = RHSSmallStorage;
280+
RHS.IsSmall = true;
269281
return;
270282
}
271283

272284
// Both a small, just swap the small elements.
273285
assert(this->isSmall() && RHS.isSmall());
274286
unsigned MinNonEmpty = std::min(this->NumNonEmpty, RHS.NumNonEmpty);
275-
std::swap_ranges(this->SmallArray, this->SmallArray + MinNonEmpty,
276-
RHS.SmallArray);
287+
std::swap_ranges(this->CurArray, this->CurArray + MinNonEmpty, RHS.CurArray);
277288
if (this->NumNonEmpty > MinNonEmpty) {
278-
std::copy(this->SmallArray + MinNonEmpty,
279-
this->SmallArray + this->NumNonEmpty,
280-
RHS.SmallArray + MinNonEmpty);
289+
std::copy(this->CurArray + MinNonEmpty, this->CurArray + this->NumNonEmpty,
290+
RHS.CurArray + MinNonEmpty);
281291
} else {
282-
std::copy(RHS.SmallArray + MinNonEmpty, RHS.SmallArray + RHS.NumNonEmpty,
283-
this->SmallArray + MinNonEmpty);
292+
std::copy(RHS.CurArray + MinNonEmpty, RHS.CurArray + RHS.NumNonEmpty,
293+
this->CurArray + MinNonEmpty);
284294
}
285295
assert(this->CurArraySize == RHS.CurArraySize);
286296
std::swap(this->NumNonEmpty, RHS.NumNonEmpty);

0 commit comments

Comments
 (0)