Skip to content

Commit 26aa20a

Browse files
rnkrnk
authored
Use range-based for to iterate over fields in record layout, NFCI (#122029)
Move the common case of FieldDecl::getFieldIndex() inline to mitigate the cost of removing the extra `FieldNo` induction variable. Also rename isNoUniqueAddress parameter to isNonVirtualBaseType, which appears to be more accurate. I think the current name is just a consequence of autocomplete gone wrong.
1 parent 9ec9287 commit 26aa20a

File tree

4 files changed

+52
-59
lines changed

4 files changed

+52
-59
lines changed

‎clang/include/clang/AST/Decl.h‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3115,8 +3115,20 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
31153115

31163116
/// Returns the index of this field within its record,
31173117
/// as appropriate for passing to ASTRecordLayout::getFieldOffset.
3118-
unsigned getFieldIndex() const;
3118+
unsigned getFieldIndex() const {
3119+
const FieldDecl *Canonical = getCanonicalDecl();
3120+
if (Canonical->CachedFieldIndex == 0) {
3121+
Canonical->setCachedFieldIndex();
3122+
assert(Canonical->CachedFieldIndex != 0);
3123+
}
3124+
return Canonical->CachedFieldIndex - 1;
3125+
}
31193126

3127+
private:
3128+
/// Set CachedFieldIndex to the index of this field plus one.
3129+
void setCachedFieldIndex() const;
3130+
3131+
public:
31203132
/// Determines whether this field is mutable (C++ only).
31213133
bool isMutable() const { return Mutable; }
31223134

‎clang/lib/AST/Decl.cpp‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4651,12 +4651,9 @@ bool FieldDecl::isPotentiallyOverlapping() const {
46514651
return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
46524652
}
46534653

4654-
unsigned FieldDecl::getFieldIndex() const {
4655-
const FieldDecl *Canonical = getCanonicalDecl();
4656-
if (Canonical != this)
4657-
return Canonical->getFieldIndex();
4658-
4659-
if (CachedFieldIndex) return CachedFieldIndex - 1;
4654+
void FieldDecl::setCachedFieldIndex() const {
4655+
assert(this == getCanonicalDecl() &&
4656+
"should be called on the canonical decl");
46604657

46614658
unsigned Index = 0;
46624659
const RecordDecl *RD = getParent()->getDefinition();
@@ -4670,7 +4667,6 @@ unsigned FieldDecl::getFieldIndex() const {
46704667
}
46714668

46724669
assert(CachedFieldIndex && "failed to find field in parent");
4673-
return CachedFieldIndex - 1;
46744670
}
46754671

46764672
SourceRange FieldDecl::getSourceRange() const {

‎clang/lib/AST/RecordLayoutBuilder.cpp‎

Lines changed: 33 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ class EmptySubobjectMap {
138138
return Offset <= MaxEmptyClassOffset;
139139
}
140140

141-
CharUnits
142-
getFieldOffset(const ASTRecordLayout &Layout, unsigned FieldNo) const {
143-
uint64_t FieldOffset = Layout.getFieldOffset(FieldNo);
141+
CharUnits getFieldOffset(const ASTRecordLayout &Layout,
142+
const FieldDecl *Field) const {
143+
uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
144144
assert(FieldOffset % CharWidth == 0 &&
145145
"Field offset not at char boundary!");
146146

@@ -298,14 +298,12 @@ EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
298298
}
299299

300300
// Traverse all member variables.
301-
unsigned FieldNo = 0;
302-
for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
303-
E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
304-
if (I->isBitField())
301+
for (const FieldDecl *Field : Info->Class->fields()) {
302+
if (Field->isBitField())
305303
continue;
306304

307-
CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
308-
if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
305+
CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
306+
if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
309307
return false;
310308
}
311309

@@ -345,14 +343,12 @@ void EmptySubobjectMap::UpdateEmptyBaseSubobjects(const BaseSubobjectInfo *Info,
345343
}
346344

347345
// Traverse all member variables.
348-
unsigned FieldNo = 0;
349-
for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
350-
E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
351-
if (I->isBitField())
346+
for (const FieldDecl *Field : Info->Class->fields()) {
347+
if (Field->isBitField())
352348
continue;
353349

354-
CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
355-
UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingEmptyBase);
350+
CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
351+
UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingEmptyBase);
356352
}
357353
}
358354

@@ -410,15 +406,12 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
410406
}
411407

412408
// Traverse all member variables.
413-
unsigned FieldNo = 0;
414-
for (CXXRecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
415-
I != E; ++I, ++FieldNo) {
416-
if (I->isBitField())
409+
for (const FieldDecl *Field : RD->fields()) {
410+
if (Field->isBitField())
417411
continue;
418412

419-
CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
420-
421-
if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
413+
CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
414+
if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
422415
return false;
423416
}
424417

@@ -465,9 +458,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
465458
return true;
466459
}
467460

468-
bool
469-
EmptySubobjectMap::CanPlaceFieldAtOffset(const FieldDecl *FD,
470-
CharUnits Offset) {
461+
bool EmptySubobjectMap::CanPlaceFieldAtOffset(const FieldDecl *FD,
462+
CharUnits Offset) {
471463
if (!CanPlaceFieldSubobjectAtOffset(FD, Offset))
472464
return false;
473465

@@ -521,15 +513,12 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
521513
}
522514

523515
// Traverse all member variables.
524-
unsigned FieldNo = 0;
525-
for (CXXRecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
526-
I != E; ++I, ++FieldNo) {
527-
if (I->isBitField())
516+
for (const FieldDecl *Field : RD->fields()) {
517+
if (Field->isBitField())
528518
continue;
529519

530-
CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
531-
532-
UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingOverlappingField);
520+
CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
521+
UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingOverlappingField);
533522
}
534523
}
535524

@@ -1455,10 +1444,8 @@ void ItaniumRecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
14551444
bool InsertExtraPadding = D->mayInsertExtraPadding(/*EmitRemark=*/true);
14561445
bool HasFlexibleArrayMember = D->hasFlexibleArrayMember();
14571446
for (auto I = D->field_begin(), End = D->field_end(); I != End; ++I) {
1458-
auto Next(I);
1459-
++Next;
1460-
LayoutField(*I,
1461-
InsertExtraPadding && (Next != End || !HasFlexibleArrayMember));
1447+
LayoutField(*I, InsertExtraPadding &&
1448+
(std::next(I) != End || !HasFlexibleArrayMember));
14621449
}
14631450
}
14641451

@@ -3672,35 +3659,33 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD,
36723659
}
36733660

36743661
// Dump fields.
3675-
uint64_t FieldNo = 0;
3676-
for (RecordDecl::field_iterator I = RD->field_begin(),
3677-
E = RD->field_end(); I != E; ++I, ++FieldNo) {
3678-
const FieldDecl &Field = **I;
3679-
uint64_t LocalFieldOffsetInBits = Layout.getFieldOffset(FieldNo);
3662+
for (const FieldDecl *Field : RD->fields()) {
3663+
uint64_t LocalFieldOffsetInBits =
3664+
Layout.getFieldOffset(Field->getFieldIndex());
36803665
CharUnits FieldOffset =
36813666
Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits);
36823667

36833668
// Recursively dump fields of record type.
3684-
if (auto RT = Field.getType()->getAs<RecordType>()) {
3669+
if (auto RT = Field->getType()->getAs<RecordType>()) {
36853670
DumpRecordLayout(OS, RT->getDecl(), C, FieldOffset, IndentLevel,
3686-
Field.getName().data(),
3671+
Field->getName().data(),
36873672
/*PrintSizeInfo=*/false,
36883673
/*IncludeVirtualBases=*/true);
36893674
continue;
36903675
}
36913676

3692-
if (Field.isBitField()) {
3677+
if (Field->isBitField()) {
36933678
uint64_t LocalFieldByteOffsetInBits = C.toBits(FieldOffset - Offset);
36943679
unsigned Begin = LocalFieldOffsetInBits - LocalFieldByteOffsetInBits;
3695-
unsigned Width = Field.getBitWidthValue(C);
3680+
unsigned Width = Field->getBitWidthValue(C);
36963681
PrintBitFieldOffset(OS, FieldOffset, Begin, Width, IndentLevel);
36973682
} else {
36983683
PrintOffset(OS, FieldOffset, IndentLevel);
36993684
}
37003685
const QualType &FieldType = C.getLangOpts().DumpRecordLayoutsCanonical
3701-
? Field.getType().getCanonicalType()
3702-
: Field.getType();
3703-
OS << FieldType << ' ' << Field << '\n';
3686+
? Field->getType().getCanonicalType()
3687+
: Field->getType();
3688+
OS << FieldType << ' ' << *Field << '\n';
37043689
}
37053690

37063691
// Dump virtual bases.

‎clang/lib/CodeGen/CGRecordLayoutBuilder.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ struct CGRecordLowering {
182182
llvm::Type *StorageType);
183183
/// Lowers an ASTRecordLayout to a llvm type.
184184
void lower(bool NonVirtualBaseType);
185-
void lowerUnion(bool isNoUniqueAddress);
185+
void lowerUnion(bool isNonVirtualBaseType);
186186
void accumulateFields(bool isNonVirtualBaseType);
187187
RecordDecl::field_iterator
188188
accumulateBitFields(bool isNonVirtualBaseType,
@@ -310,9 +310,9 @@ void CGRecordLowering::lower(bool NVBaseType) {
310310
computeVolatileBitfields();
311311
}
312312

313-
void CGRecordLowering::lowerUnion(bool isNoUniqueAddress) {
313+
void CGRecordLowering::lowerUnion(bool isNonVirtualBaseType) {
314314
CharUnits LayoutSize =
315-
isNoUniqueAddress ? Layout.getDataSize() : Layout.getSize();
315+
isNonVirtualBaseType ? Layout.getDataSize() : Layout.getSize();
316316
llvm::Type *StorageType = nullptr;
317317
bool SeenNamedMember = false;
318318
// Iterate through the fields setting bitFieldInfo and the Fields array. Also

0 commit comments

Comments
 (0)