[CIR] Add support for derived class declarations #142823
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds the minimal support for declaring a pointer to a derived class. This includes only the changes necessary to compute the record layout for the derived class and declare a variable that points to it. Support for accessing members of either the derived or base class is deferred until a later change, as is support for declaring a variable that is an instance of the derived class.
andykaylor
requested review from
xlauko,
bcardosolopes,
mmha,
erichkeane and
lanza
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis adds the minimal support for declaring a pointer to a derived class. This includes only the changes necessary to compute the record layout for the derived class and declare a variable that points to it. Support for accessing members of either the derived or base class is deferred until a later change, as is support for declaring a variable that is an instance of the derived class. Full diff: https://github.com/llvm/llvm-project/pull/142823.diff 7 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
index 75eb9cc045a48..b29b703c47139 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -474,6 +474,18 @@ def CIR_RecordType : CIR_Type<"Record", "record",
let genVerifyDecl = 1;
let builders = [
+ // Create an identified and complete record type.
+ TypeBuilder<(ins
+ "llvm::ArrayRef<mlir::Type>":$members,
+ "mlir::StringAttr":$name,
+ "bool":$packed,
+ "bool":$padded,
+ "RecordKind":$kind
+ ), [{
+ return $_get($_ctxt, members, name, /*complete=*/true, packed, padded,
+ kind);
+ }]>,
+
// Create an identified and incomplete record type.
TypeBuilder<(ins
"mlir::StringAttr":$name,
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 2a7cd464b8f6b..64e5acd830ae5 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -136,6 +136,7 @@ struct MissingFeatures {
static bool cxxSupport() { return false; }
static bool recordZeroInit() { return false; }
static bool zeroSizeRecordMembers() { return false; }
+ static bool recordLayoutVirtualBases() { return false; }
// CXXABI
static bool cxxABI() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index 5f17b5d58acaa..717712150f749 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -96,6 +96,28 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
llvm_unreachable("Unsupported record kind");
}
+ /// Get a CIR named record type.
+ ///
+ /// If a record already exists and is complete, but the client tries to fetch
+ /// it with a different set of attributes, this method will crash.
+ cir::RecordType getCompleteRecordTy(llvm::ArrayRef<mlir::Type> members,
+ llvm::StringRef name, bool packed,
+ bool padded) {
+ const auto nameAttr = getStringAttr(name);
+ auto kind = cir::RecordType::RecordKind::Struct;
+ assert(!cir::MissingFeatures::astRecordDeclAttr());
+
+ // Create or get the record.
+ auto type =
+ getType<cir::RecordType>(members, nameAttr, packed, padded, kind);
+
+ // Complete an incomplete record or ensure the existing complete record
+ // matches the requested attributes.
+ type.complete(members, packed, padded);
+
+ return type;
+ }
+
/// Get an incomplete CIR struct type. If we have a complete record
/// declaration, we may create an incomplete type and then add the
/// members, so \p rd here may be complete.
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
index 2ece85b8aa0a3..ac8832b8c9b24 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
@@ -29,10 +29,18 @@ class CIRGenRecordLayout {
/// as a complete object.
cir::RecordType completeObjectType;
+ /// The CIR type for the non-virtual part of this record layout; used when
+ /// laying it out as a base subobject.
+ cir::RecordType baseSubobjectType;
+
/// Map from (non-bit-field) record field to the corresponding cir record type
/// field no. This info is populated by the record builder.
llvm::DenseMap<const clang::FieldDecl *, unsigned> fieldIdxMap;
+ // FIXME: Maybe we could use CXXBaseSpecifier as the key and use a single map
+ // for both virtual and non-virtual bases.
+ llvm::DenseMap<const clang::CXXRecordDecl *, unsigned> nonVirtualBases;
+
/// False if any direct or indirect subobject of this class, when considered
/// as a complete object, requires a non-zero bitpattern when
/// zero-initialized.
@@ -45,9 +53,11 @@ class CIRGenRecordLayout {
unsigned zeroInitializableAsBase : 1;
public:
- CIRGenRecordLayout(cir::RecordType completeObjectType, bool zeroInitializable,
+ CIRGenRecordLayout(cir::RecordType completeObjectType,
+ cir::RecordType baseSubobjectType, bool zeroInitializable,
bool zeroInitializableAsBase)
: completeObjectType(completeObjectType),
+ baseSubobjectType(baseSubobjectType),
zeroInitializable(zeroInitializable),
zeroInitializableAsBase(zeroInitializableAsBase) {}
@@ -55,6 +65,10 @@ class CIRGenRecordLayout {
/// this record.
cir::RecordType getCIRType() const { return completeObjectType; }
+ /// Return the "base subobject" LLVM type associated with
+ /// this record.
+ cir::RecordType getBaseSubobjectCIRType() const { return baseSubobjectType; }
+
/// Return cir::RecordType element number that corresponds to the field FD.
unsigned getCIRFieldNo(const clang::FieldDecl *fd) const {
fd = fd->getCanonicalDecl();
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 53aa0aee36fc3..0aeef7fd89aef 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -40,15 +40,18 @@ struct CIRRecordLowering final {
// member type that ensures correct rounding.
struct MemberInfo final {
CharUnits offset;
- enum class InfoKind { Field } kind;
+ enum class InfoKind { Field, Base } kind;
mlir::Type data;
union {
const FieldDecl *fieldDecl;
- // CXXRecordDecl will be used here when base types are supported.
+ const CXXRecordDecl *cxxRecordDecl;
};
MemberInfo(CharUnits offset, InfoKind kind, mlir::Type data,
const FieldDecl *fieldDecl = nullptr)
- : offset(offset), kind(kind), data(data), fieldDecl(fieldDecl) {};
+ : offset{offset}, kind{kind}, data{data}, fieldDecl{fieldDecl} {}
+ MemberInfo(CharUnits offset, InfoKind kind, mlir::Type data,
+ const CXXRecordDecl *rd)
+ : offset{offset}, kind{kind}, data{data}, cxxRecordDecl{rd} {}
// MemberInfos are sorted so we define a < operator.
bool operator<(const MemberInfo &other) const {
return offset < other.offset;
@@ -71,6 +74,8 @@ struct CIRRecordLowering final {
/// Inserts padding everywhere it's needed.
void insertPadding();
+ void accumulateBases(const CXXRecordDecl *cxxRecordDecl);
+ void accumulateVPtrs();
void accumulateFields();
CharUnits bitsToCharUnits(uint64_t bitOffset) {
@@ -89,6 +94,9 @@ struct CIRRecordLowering final {
bool isZeroInitializable(const FieldDecl *fd) {
return cirGenTypes.isZeroInitializable(fd->getType());
}
+ bool isZeroInitializable(const RecordDecl *rd) {
+ return cirGenTypes.isZeroInitializable(rd);
+ }
/// Wraps cir::IntType with some implicit arguments.
mlir::Type getUIntNType(uint64_t numBits) {
@@ -112,6 +120,11 @@ struct CIRRecordLowering final {
: cir::ArrayType::get(type, numberOfChars.getQuantity());
}
+ // Gets the CIR BaseSubobject type from a CXXRecordDecl.
+ mlir::Type getStorageType(const CXXRecordDecl *RD) {
+ return cirGenTypes.getCIRGenRecordLayout(RD).getBaseSubobjectCIRType();
+ }
+
mlir::Type getStorageType(const FieldDecl *fieldDecl) {
mlir::Type type = cirGenTypes.convertTypeForMem(fieldDecl->getType());
if (fieldDecl->isBitField()) {
@@ -145,6 +158,7 @@ struct CIRRecordLowering final {
// Output fields, consumed by CIRGenTypes::computeRecordLayout
llvm::SmallVector<mlir::Type, 16> fieldTypes;
llvm::DenseMap<const FieldDecl *, unsigned> fieldIdxMap;
+ llvm::DenseMap<const CXXRecordDecl *, unsigned> nonVirtualBases;
cir::CIRDataLayout dataLayout;
LLVM_PREFERRED_TYPE(bool)
@@ -179,24 +193,20 @@ void CIRRecordLowering::lower() {
return;
}
- assert(!cir::MissingFeatures::cxxSupport());
-
+ assert(!cir::MissingFeatures::recordLayoutVirtualBases());
CharUnits size = astRecordLayout.getSize();
accumulateFields();
if (const auto *cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) {
- if (cxxRecordDecl->getNumBases() > 0) {
- CIRGenModule &cgm = cirGenTypes.getCGModule();
- cgm.errorNYI(recordDecl->getSourceRange(),
- "CIRRecordLowering::lower: derived CXXRecordDecl");
- return;
- }
+ accumulateVPtrs();
+ accumulateBases(cxxRecordDecl);
if (members.empty()) {
appendPaddingBytes(size);
assert(!cir::MissingFeatures::bitfields());
return;
}
+ assert(!cir::MissingFeatures::recordLayoutVirtualBases());
}
llvm::stable_sort(members);
@@ -223,8 +233,10 @@ void CIRRecordLowering::fillOutputFields() {
fieldTypes.size() - 1;
// A field without storage must be a bitfield.
assert(!cir::MissingFeatures::bitfields());
+ } else if (member.kind == MemberInfo::InfoKind::Base) {
+ nonVirtualBases[member.cxxRecordDecl] = fieldTypes.size() - 1;
}
- assert(!cir::MissingFeatures::cxxSupport());
+ assert(!cir::MissingFeatures::recordLayoutVirtualBases());
}
}
@@ -254,9 +266,14 @@ void CIRRecordLowering::calculateZeroInit() {
continue;
zeroInitializable = zeroInitializableAsBase = false;
return;
+ } else if (member.kind == MemberInfo::InfoKind::Base) {
+ if (isZeroInitializable(member.cxxRecordDecl))
+ continue;
+ zeroInitializable = false;
+ if (member.kind == MemberInfo::InfoKind::Base)
+ zeroInitializableAsBase = false;
}
- // TODO(cir): handle base types
- assert(!cir::MissingFeatures::cxxSupport());
+ assert(!cir::MissingFeatures::recordLayoutVirtualBases());
}
}
@@ -317,6 +334,27 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
lowering.lower();
// If we're in C++, compute the base subobject type.
+ cir::RecordType baseTy;
+ if (llvm::isa<CXXRecordDecl>(rd) && !rd->isUnion() &&
+ !rd->hasAttr<FinalAttr>()) {
+ baseTy = *ty;
+ if (lowering.astRecordLayout.getNonVirtualSize() !=
+ lowering.astRecordLayout.getSize()) {
+ CIRRecordLowering baseLowering(*this, rd, /*Packed=*/lowering.packed);
+ baseLowering.lower();
+ std::string baseIdentifier = getRecordTypeName(rd, ".base");
+ baseTy =
+ builder.getCompleteRecordTy(baseLowering.fieldTypes, baseIdentifier,
+ baseLowering.packed, baseLowering.padded);
+ // TODO(cir): add something like addRecordTypeName
+
+ // BaseTy and Ty must agree on their packedness for getCIRFieldNo to work
+ // on both of them with the same index.
+ assert(lowering.packed == baseLowering.packed &&
+ "Non-virtual and complete types must agree on packedness");
+ }
+ }
+
if (llvm::isa<CXXRecordDecl>(rd) && !rd->isUnion() &&
!rd->hasAttr<FinalAttr>()) {
if (lowering.astRecordLayout.getNonVirtualSize() !=
@@ -332,10 +370,13 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
ty->complete(lowering.fieldTypes, lowering.packed, lowering.padded);
auto rl = std::make_unique<CIRGenRecordLayout>(
- ty ? *ty : cir::RecordType(), (bool)lowering.zeroInitializable,
- (bool)lowering.zeroInitializableAsBase);
+ ty ? *ty : cir::RecordType{}, baseTy ? baseTy : cir::RecordType{},
+ (bool)lowering.zeroInitializable, (bool)lowering.zeroInitializableAsBase);
assert(!cir::MissingFeatures::recordZeroInit());
+
+ rl->nonVirtualBases.swap(lowering.nonVirtualBases);
+
assert(!cir::MissingFeatures::cxxSupport());
assert(!cir::MissingFeatures::bitfields());
@@ -415,3 +456,38 @@ void CIRRecordLowering::lowerUnion() {
if (layoutSize % getAlignment(storageType))
packed = true;
}
+
+void CIRRecordLowering::accumulateBases(const CXXRecordDecl *cxxRecordDecl) {
+ // If we've got a primary virtual base, we need to add it with the bases.
+ if (astRecordLayout.isPrimaryBaseVirtual()) {
+ cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
+ "accumulateBases: primary virtual base");
+ }
+
+ // Accumulate the non-virtual bases.
+ for ([[maybe_unused]] const auto &base : cxxRecordDecl->bases()) {
+ if (base.isVirtual()) {
+ cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
+ "accumulateBases: virtual base");
+ continue;
+ }
+ // Bases can be zero-sized even if not technically empty if they
+ // contain only a trailing array member.
+ const CXXRecordDecl *baseDecl = base.getType()->getAsCXXRecordDecl();
+ if (!baseDecl->isEmpty() &&
+ !astContext.getASTRecordLayout(baseDecl).getNonVirtualSize().isZero()) {
+ members.push_back(MemberInfo(astRecordLayout.getBaseClassOffset(baseDecl),
+ MemberInfo::InfoKind::Base,
+ getStorageType(baseDecl), baseDecl));
+ }
+ }
+}
+
+void CIRRecordLowering::accumulateVPtrs() {
+ if (astRecordLayout.hasOwnVFPtr())
+ cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
+ "accumulateVPtrs: hasOwnVFPtr");
+ if (astRecordLayout.hasOwnVBPtr())
+ cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
+ "accumulateVPtrs: hasOwnVBPtr");
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index d962372a4bcc0..f748649c8ac70 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -239,9 +239,10 @@ mlir::Type CIRGenTypes::convertRecordDeclType(const clang::RecordDecl *rd) {
// Force conversion of non-virtual base classes recursively.
if (const auto *cxxRecordDecl = dyn_cast<CXXRecordDecl>(rd)) {
- if (cxxRecordDecl->getNumBases() > 0) {
- cgm.errorNYI(rd->getSourceRange(),
- "convertRecordDeclType: derived CXXRecordDecl");
+ for (const auto &base : cxxRecordDecl->bases()) {
+ if (base.isVirtual())
+ continue;
+ convertRecordDeclType(base.getType()->castAs<RecordType>()->getDecl());
}
}
diff --git a/clang/test/CIR/CodeGen/class.cpp b/clang/test/CIR/CodeGen/class.cpp
index dd280d3d906ba..7c41bdb7112e9 100644
--- a/clang/test/CIR/CodeGen/class.cpp
+++ b/clang/test/CIR/CodeGen/class.cpp
@@ -6,13 +6,19 @@
// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
// CIR: !rec_IncompleteC = !cir.record<class "IncompleteC" incomplete>
+// CIR: !rec_Base = !cir.record<class "Base" {!s32i}>
// CIR: !rec_CompleteC = !cir.record<class "CompleteC" {!s32i, !s8i}>
+// CIR: !rec_Derived = !cir.record<class "Derived" {!rec_Base, !s32i}>
// Note: LLVM and OGCG do not emit the type for incomplete classes.
// LLVM: %class.CompleteC = type { i32, i8 }
+// LLVM: %class.Derived = type { %class.Base, i32 }
+// LLVM: %class.Base = type { i32 }
// OGCG: %class.CompleteC = type { i32, i8 }
+// OGCG: %class.Derived = type { %class.Base, i32 }
+// OGCG: %class.Base = type { i32 }
class IncompleteC;
IncompleteC *p;
@@ -32,3 +38,16 @@ CompleteC cc;
// CIR: cir.global external @cc = #cir.zero : !rec_CompleteC
// LLVM: @cc = global %class.CompleteC zeroinitializer
// OGCG: @cc = global %class.CompleteC zeroinitializer
+
+class Base {
+public:
+ int a;
+};
+
+class Derived : public Base {
+public:
+ int b;
+};
+
+int use(Derived *d) { return d->b; }
+
|
erichkeane
reviewed
Jun 4, 2025
| @@ -96,6 +96,28 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy { | |||
| llvm_unreachable("Unsupported record kind"); | |||
| } | |||
|
|
|||
| /// Get a CIR named record type. | |||
| /// | |||
| /// If a record already exists and is complete, but the client tries to fetch | |||
There was a problem hiding this comment.
Can we have some asserts here for this condition?
| int b; | ||
| }; | ||
|
|
||
| int use(Derived *d) { return d->b; } |
There was a problem hiding this comment.
Can we get a CRTP test as well? Something like:
template<typename Derived>
struct CRTP {
Derived *otherDerived /* initializer left out, but add something to make sure this isn't ub */;
void callThing() {
static_cast<Derived>(*this)->thing();
otherDerived->thing();
}
};
struct D : CRTP<D> {
void thing(){}
};
void use( D&d) {
d.callThing();
}
I suspect it'll work fine, but its awkward enough with this sorta thing that it might be demonstrative.
There was a problem hiding this comment.
We can't handle this yet because it requires an implicit DerivedToBase cast, which will be in an upcoming patch.
There was a problem hiding this comment.
Ah, hrmph. Well, file this one away then :)
bcardosolopes
approved these changes
Jun 4, 2025
erichkeane
approved these changes
Jun 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the minimal support for declaring a pointer to a derived class. This includes only the changes necessary to compute the record layout for the derived class and declare a variable that points to it.
Support for accessing members of either the derived or base class is deferred until a later change, as is support for declaring a variable that is an instance of the derived class.