Skip to content

Commit a8a494f

Browse files
nikicnikic
authored
[SmallPtrSet] Remove SmallArray member (NFC) (#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.
1 parent a348f22 commit a8a494f

File tree

2 files changed

+78
-63
lines changed

2 files changed

+78
-63
lines changed

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

Lines changed: 34 additions & 29 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();
@@ -219,8 +219,8 @@ class SmallPtrSetImplBase : public DebugEpochBase {
219219
bool contains_imp(const void *Ptr) const {
220220
if (isSmall()) {
221221
// Linear search for the item.
222-
const void *const *APtr = SmallArray;
223-
const void *const *E = SmallArray + NumNonEmpty;
222+
const void *const *APtr = CurArray;
223+
const void *const *E = CurArray + NumNonEmpty;
224224
for (; APtr != E; ++APtr)
225225
if (*APtr == Ptr)
226226
return true;
@@ -230,7 +230,7 @@ class SmallPtrSetImplBase : public DebugEpochBase {
230230
return doFind(Ptr) != nullptr;
231231
}
232232

233-
bool isSmall() const { return CurArray == SmallArray; }
233+
bool isSmall() const { return IsSmall; }
234234

235235
private:
236236
std::pair<const void *const *, bool> insert_imp_big(const void *Ptr);
@@ -245,16 +245,19 @@ class SmallPtrSetImplBase : public DebugEpochBase {
245245
protected:
246246
/// swap - Swaps the elements of two sets.
247247
/// Note: This method assumes that both sets have the same small size.
248-
void swap(SmallPtrSetImplBase &RHS);
248+
void swap(const void **SmallStorage, const void **RHSSmallStorage,
249+
SmallPtrSetImplBase &RHS);
249250

250-
void CopyFrom(const SmallPtrSetImplBase &RHS);
251-
void MoveFrom(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
251+
void copyFrom(const void **SmallStorage, const SmallPtrSetImplBase &RHS);
252+
void moveFrom(const void **SmallStorage, unsigned SmallSize,
253+
const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
252254

253255
private:
254-
/// Code shared by MoveFrom() and move constructor.
255-
void MoveHelper(unsigned SmallSize, SmallPtrSetImplBase &&RHS);
256-
/// Code shared by CopyFrom() and copy constructor.
257-
void CopyHelper(const SmallPtrSetImplBase &RHS);
256+
/// Code shared by moveFrom() and move constructor.
257+
void moveHelper(const void **SmallStorage, unsigned SmallSize,
258+
const void **RHSSmallStorage, SmallPtrSetImplBase &&RHS);
259+
/// Code shared by copyFrom() and copy constructor.
260+
void copyHelper(const SmallPtrSetImplBase &RHS);
258261
};
259262

260263
/// SmallPtrSetIteratorImpl - This is the common base class shared between all
@@ -415,7 +418,7 @@ class SmallPtrSetImpl : public SmallPtrSetImplBase {
415418
bool remove_if(UnaryPredicate P) {
416419
bool Removed = false;
417420
if (isSmall()) {
418-
const void **APtr = SmallArray, **E = SmallArray + NumNonEmpty;
421+
const void **APtr = CurArray, **E = CurArray + NumNonEmpty;
419422
while (APtr != E) {
420423
PtrType Ptr = PtrTraits::getFromVoidPointer(const_cast<void *>(*APtr));
421424
if (P(Ptr)) {
@@ -540,7 +543,8 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
540543
SmallPtrSet() : BaseT(SmallStorage, SmallSizePowTwo) {}
541544
SmallPtrSet(const SmallPtrSet &that) : BaseT(SmallStorage, that) {}
542545
SmallPtrSet(SmallPtrSet &&that)
543-
: BaseT(SmallStorage, SmallSizePowTwo, std::move(that)) {}
546+
: BaseT(SmallStorage, SmallSizePowTwo, that.SmallStorage,
547+
std::move(that)) {}
544548

545549
template<typename It>
546550
SmallPtrSet(It I, It E) : BaseT(SmallStorage, SmallSizePowTwo) {
@@ -555,14 +559,15 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
555559
SmallPtrSet<PtrType, SmallSize> &
556560
operator=(const SmallPtrSet<PtrType, SmallSize> &RHS) {
557561
if (&RHS != this)
558-
this->CopyFrom(RHS);
562+
this->copyFrom(SmallStorage, RHS);
559563
return *this;
560564
}
561565

562566
SmallPtrSet<PtrType, SmallSize> &
563567
operator=(SmallPtrSet<PtrType, SmallSize> &&RHS) {
564568
if (&RHS != this)
565-
this->MoveFrom(SmallSizePowTwo, std::move(RHS));
569+
this->moveFrom(SmallStorage, SmallSizePowTwo, RHS.SmallStorage,
570+
std::move(RHS));
566571
return *this;
567572
}
568573

@@ -575,7 +580,7 @@ class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
575580

576581
/// swap - Swaps the elements of two sets.
577582
void swap(SmallPtrSet<PtrType, SmallSize> &RHS) {
578-
SmallPtrSetImplBase::swap(RHS);
583+
SmallPtrSetImplBase::swap(SmallStorage, RHS.SmallStorage, RHS);
579584
}
580585
};
581586

‎llvm/lib/Support/SmallPtrSet.cpp‎

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -134,32 +134,33 @@ 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

151151
// Copy over the that array.
152-
CopyHelper(that);
152+
copyHelper(that);
153153
}
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,12 +182,13 @@ void SmallPtrSetImplBase::CopyFrom(const SmallPtrSetImplBase &RHS) {
180182
sizeof(void*) * RHS.CurArraySize);
181183
CurArray = T;
182184
}
185+
IsSmall = false;
183186
}
184187

185-
CopyHelper(RHS);
188+
copyHelper(RHS);
186189
}
187190

188-
void SmallPtrSetImplBase::CopyHelper(const SmallPtrSetImplBase &RHS) {
191+
void SmallPtrSetImplBase::copyHelper(const SmallPtrSetImplBase &RHS) {
189192
// Copy over the new array size
190193
CurArraySize = RHS.CurArraySize;
191194

@@ -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)