https://github.com/rnk created https://github.com/llvm/llvm-project/pull/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.
>From 1824674744d6fba1dd74c74d54aae9b603d8b854 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <r...@google.com>
Date: Wed, 8 Jan 2025 00:24:09 +0000
Subject: [PATCH] Use range-based for to iterate over fields in record layout,
NFC
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.
---
clang/include/clang/AST/Decl.h | 14 +++-
clang/lib/AST/Decl.cpp | 10 +--
clang/lib/AST/RecordLayoutBuilder.cpp | 73 ++++++++-------------
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 6 +-
4 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..7f6c1867bb8c10 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3115,7 +3115,19 @@ class FieldDecl : public DeclaratorDecl, public
Mergeable<FieldDecl> {
/// Returns the index of this field within its record,
/// as appropriate for passing to ASTRecordLayout::getFieldOffset.
- unsigned getFieldIndex() const;
+ unsigned getFieldIndex() const {
+ const FieldDecl *C = getCanonicalDecl();
+ if (C->CachedFieldIndex == 0)
+ C->setCachedFieldIndex();
+ assert(C->CachedFieldIndex);
+ return C->CachedFieldIndex - 1;
+ }
+
+private:
+ /// Set CachedFieldIndex to the index of this field plus one.
+ void setCachedFieldIndex() const;
+
+public:
/// Determines whether this field is mutable (C++ only).
bool isMutable() const { return Mutable; }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 741e908cf9bc56..97e23dd1aaa920 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4651,12 +4651,9 @@ bool FieldDecl::isPotentiallyOverlapping() const {
return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
}
-unsigned FieldDecl::getFieldIndex() const {
- const FieldDecl *Canonical = getCanonicalDecl();
- if (Canonical != this)
- return Canonical->getFieldIndex();
-
- if (CachedFieldIndex) return CachedFieldIndex - 1;
+void FieldDecl::setCachedFieldIndex() const {
+ assert(this == getCanonicalDecl() &&
+ "should be called on the canonical decl");
unsigned Index = 0;
const RecordDecl *RD = getParent()->getDefinition();
@@ -4670,7 +4667,6 @@ unsigned FieldDecl::getFieldIndex() const {
}
assert(CachedFieldIndex && "failed to find field in parent");
- return CachedFieldIndex - 1;
}
SourceRange FieldDecl::getSourceRange() const {
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index f749d3a705fc99..b287a7d5f60451 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -139,8 +139,8 @@ class EmptySubobjectMap {
}
CharUnits
- getFieldOffset(const ASTRecordLayout &Layout, unsigned FieldNo) const {
- uint64_t FieldOffset = Layout.getFieldOffset(FieldNo);
+ getFieldOffset(const ASTRecordLayout &Layout, const FieldDecl *Field) const {
+ uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
assert(FieldOffset % CharWidth == 0 &&
"Field offset not at char boundary!");
@@ -298,14 +298,12 @@ EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const
BaseSubobjectInfo *Info,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
- E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : Info->Class->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
- if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
return false;
}
@@ -345,14 +343,12 @@ void EmptySubobjectMap::UpdateEmptyBaseSubobjects(const
BaseSubobjectInfo *Info,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
- E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : Info->Class->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
- UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingEmptyBase);
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingEmptyBase);
}
}
@@ -410,15 +406,12 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const
CXXRecordDecl *RD,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = RD->field_begin(), E =
RD->field_end();
- I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : RD->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
-
- if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
return false;
}
@@ -521,15 +514,12 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = RD->field_begin(), E =
RD->field_end();
- I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : RD->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
-
- UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingOverlappingField);
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingOverlappingField);
}
}
@@ -1455,10 +1445,8 @@ void ItaniumRecordLayoutBuilder::LayoutFields(const
RecordDecl *D) {
bool InsertExtraPadding = D->mayInsertExtraPadding(/*EmitRemark=*/true);
bool HasFlexibleArrayMember = D->hasFlexibleArrayMember();
for (auto I = D->field_begin(), End = D->field_end(); I != End; ++I) {
- auto Next(I);
- ++Next;
- LayoutField(*I,
- InsertExtraPadding && (Next != End ||
!HasFlexibleArrayMember));
+ LayoutField(*I, InsertExtraPadding &&
+ (std::next(I) != End || !HasFlexibleArrayMember));
}
}
@@ -3672,35 +3660,32 @@ static void DumpRecordLayout(raw_ostream &OS, const
RecordDecl *RD,
}
// Dump fields.
- uint64_t FieldNo = 0;
- for (RecordDecl::field_iterator I = RD->field_begin(),
- E = RD->field_end(); I != E; ++I, ++FieldNo) {
- const FieldDecl &Field = **I;
- uint64_t LocalFieldOffsetInBits = Layout.getFieldOffset(FieldNo);
+ for (const FieldDecl *Field : RD->fields()) {
+ uint64_t LocalFieldOffsetInBits =
Layout.getFieldOffset(Field->getFieldIndex());
CharUnits FieldOffset =
Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits);
// Recursively dump fields of record type.
- if (auto RT = Field.getType()->getAs<RecordType>()) {
+ if (auto RT = Field->getType()->getAs<RecordType>()) {
DumpRecordLayout(OS, RT->getDecl(), C, FieldOffset, IndentLevel,
- Field.getName().data(),
+ Field->getName().data(),
/*PrintSizeInfo=*/false,
/*IncludeVirtualBases=*/true);
continue;
}
- if (Field.isBitField()) {
+ if (Field->isBitField()) {
uint64_t LocalFieldByteOffsetInBits = C.toBits(FieldOffset - Offset);
unsigned Begin = LocalFieldOffsetInBits - LocalFieldByteOffsetInBits;
- unsigned Width = Field.getBitWidthValue(C);
+ unsigned Width = Field->getBitWidthValue(C);
PrintBitFieldOffset(OS, FieldOffset, Begin, Width, IndentLevel);
} else {
PrintOffset(OS, FieldOffset, IndentLevel);
}
const QualType &FieldType = C.getLangOpts().DumpRecordLayoutsCanonical
- ? Field.getType().getCanonicalType()
- : Field.getType();
- OS << FieldType << ' ' << Field << '\n';
+ ? Field->getType().getCanonicalType()
+ : Field->getType();
+ OS << FieldType << ' ' << *Field << '\n';
}
// Dump virtual bases.
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index ea44e6f21f3c86..209ef236f0d5dc 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -182,7 +182,7 @@ struct CGRecordLowering {
llvm::Type *StorageType);
/// Lowers an ASTRecordLayout to a llvm type.
void lower(bool NonVirtualBaseType);
- void lowerUnion(bool isNoUniqueAddress);
+ void lowerUnion(bool isNonVirtualBaseType);
void accumulateFields(bool isNonVirtualBaseType);
RecordDecl::field_iterator
accumulateBitFields(bool isNonVirtualBaseType,
@@ -310,9 +310,9 @@ void CGRecordLowering::lower(bool NVBaseType) {
computeVolatileBitfields();
}
-void CGRecordLowering::lowerUnion(bool isNoUniqueAddress) {
+void CGRecordLowering::lowerUnion(bool isNonVirtualBaseType) {
CharUnits LayoutSize =
- isNoUniqueAddress ? Layout.getDataSize() : Layout.getSize();
+ isNonVirtualBaseType ? Layout.getDataSize() : Layout.getSize();
llvm::Type *StorageType = nullptr;
bool SeenNamedMember = false;
// Iterate through the fields setting bitFieldInfo and the Fields array. Also
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
