-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CIR] Add support for non-virtual base class initialization #148080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change adds support for initializing non-virtual base classes during the prologue of a derived class' constructor.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis change adds support for initializing non-virtual base classes during the prologue of a derived class' constructor. Full diff: https://github.com/llvm/llvm-project/pull/148080.diff 5 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
index da8166a596d42..cc4a615dc392e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
@@ -117,6 +117,75 @@ static void emitMemberInitializer(CIRGenFunction &cgf,
cgf.emitInitializerForField(field, lhs, memberInit->getInit());
}
+static bool isInitializerOfDynamicClass(const CXXCtorInitializer *baseInit) {
+ const Type *baseType = baseInit->getBaseClass();
+ const auto *baseClassDecl =
+ cast<CXXRecordDecl>(baseType->castAs<RecordType>()->getDecl());
+ return baseClassDecl->isDynamicClass();
+}
+
+/// Gets the address of a direct base class within a complete object.
+/// This should only be used for (1) non-virtual bases or (2) virtual bases
+/// when the type is known to be complete (e.g. in complete destructors).
+///
+/// The object pointed to by 'thisAddr' is assumed to be non-null.
+Address CIRGenFunction::getAddressOfDirectBaseInCompleteClass(
+ mlir::Location loc, Address thisAddr, const CXXRecordDecl *derived,
+ const CXXRecordDecl *base, bool baseIsVirtual) {
+ // 'thisAddr' must be a pointer (in some address space) to Derived.
+ assert(thisAddr.getElementType() == convertType(derived));
+
+ // Compute the offset of the virtual base.
+ CharUnits offset;
+ const ASTRecordLayout &layout = getContext().getASTRecordLayout(derived);
+ if (baseIsVirtual)
+ offset = layout.getVBaseClassOffset(base);
+ else
+ offset = layout.getBaseClassOffset(base);
+
+ return builder.createBaseClassAddr(loc, thisAddr, convertType(base),
+ offset.getQuantity(),
+ /*assumeNotNull=*/true);
+}
+
+void CIRGenFunction::emitBaseInitializer(mlir::Location loc,
+ const CXXRecordDecl *classDecl,
+ CXXCtorInitializer *baseInit) {
+ assert(curFuncDecl && "loading 'this' without a func declaration?");
+ assert(isa<CXXMethodDecl>(curFuncDecl));
+
+ assert(baseInit->isBaseInitializer() && "Must have base initializer!");
+
+ Address thisPtr = loadCXXThisAddress();
+
+ const Type *baseType = baseInit->getBaseClass();
+ const auto *baseClassDecl =
+ cast<CXXRecordDecl>(baseType->castAs<RecordType>()->getDecl());
+
+ bool isBaseVirtual = baseInit->isBaseVirtual();
+
+ // If the initializer for the base (other than the constructor
+ // itself) accesses 'this' in any way, we need to initialize the
+ // vtables.
+ if (classDecl->isDynamicClass()) {
+ cgm.errorNYI(loc, "emitBaseInitializer: dynamic class");
+ return;
+ }
+
+ // We can pretend to be a complete class because it only matters for
+ // virtual bases, and we only do virtual bases for complete ctors.
+ Address v = getAddressOfDirectBaseInCompleteClass(
+ loc, thisPtr, classDecl, baseClassDecl, isBaseVirtual);
+ assert(!cir::MissingFeatures::aggValueSlotGC());
+ AggValueSlot aggSlot = AggValueSlot::forAddr(
+ v, Qualifiers(), AggValueSlot::IsDestructed, AggValueSlot::IsNotAliased,
+ getOverlapForBaseInit(classDecl, baseClassDecl, isBaseVirtual));
+
+ emitAggExpr(baseInit->getInit(), aggSlot);
+
+ assert(!cir::MissingFeatures::requiresCleanups());
+}
+
/// This routine generates necessary code to initialize base classes and
/// non-static data members belonging to this constructor.
void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
@@ -154,12 +223,29 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
return;
}
- if ((*b)->isBaseInitializer()) {
+ const mlir::Value oldThisValue = cxxThisValue;
+ if (!constructVBases && (*b)->isBaseInitializer() && (*b)->isBaseVirtual()) {
cgm.errorNYI(cd->getSourceRange(),
- "emitCtorPrologue: non-virtual base initializer");
+ "emitCtorPrologue: virtual base initializer");
return;
}
+ // Handle non-virtual base initializers.
+ for (; b != e && (*b)->isBaseInitializer(); b++) {
+ assert(!(*b)->isBaseVirtual());
+
+ if (cgm.getCodeGenOpts().StrictVTablePointers &&
+ cgm.getCodeGenOpts().OptimizationLevel > 0 &&
+ isInitializerOfDynamicClass(*b)) {
+ cgm.errorNYI(cd->getSourceRange(),
+ "emitCtorPrologue: strict vtable pointers");
+ return;
+ }
+ emitBaseInitializer(getLoc(cd->getBeginLoc()), classDecl, *b);
+ }
+
+ cxxThisValue = oldThisValue;
+
if (classDecl->isDynamicClass()) {
cgm.errorNYI(cd->getSourceRange(),
"emitCtorPrologue: initialize vtable pointers");
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 300ba7a456e4b..f2d71b4fb1c08 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -1578,10 +1578,15 @@ void CIRGenFunction::emitCXXConstructExpr(const CXXConstructExpr *e,
delegating = true;
break;
case CXXConstructionKind::VirtualBase:
- case CXXConstructionKind::NonVirtualBase:
+ // This should just set 'forVirtualBase' to true and fall through, but
+ // virtual base class support is otherwise missing, so this needs to wait
+ // until it can be tested.
cgm.errorNYI(e->getSourceRange(),
- "emitCXXConstructExpr: other construction kind");
+ "emitCXXConstructExpr: virtual base constructor");
return;
+ case CXXConstructionKind::NonVirtualBase:
+ type = Ctor_Base;
+ break;
}
emitCXXConstructorCall(cd, type, forVirtualBase, delegating, dest, e);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
index ffe1b701b244e..0d12c5c3edded 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
@@ -16,6 +16,7 @@
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/StmtVisitor.h"
#include <cstdint>
@@ -362,6 +363,28 @@ void AggExprEmitter::visitCXXParenListOrInitListExpr(
"visitCXXParenListOrInitListExpr Record or VariableSizeArray type");
}
+// TODO(cir): This could be shared with classic codegen.
+AggValueSlot::Overlap_t CIRGenFunction::getOverlapForBaseInit(
+ const CXXRecordDecl *rd, const CXXRecordDecl *baseRD, bool isVirtual) {
+ // If the most-derived object is a field declared with [[no_unique_address]],
+ // the tail padding of any virtual base could be reused for other subobjects
+ // of that field's class.
+ if (isVirtual)
+ return AggValueSlot::MayOverlap;
+
+ // If the base class is laid out entirely within the nvsize of the derived
+ // class, its tail padding cannot yet be initialized, so we can issue
+ // stores at the full width of the base class.
+ const ASTRecordLayout &layout = getContext().getASTRecordLayout(rd);
+ if (layout.getBaseClassOffset(baseRD) +
+ getContext().getASTRecordLayout(baseRD).getSize() <=
+ layout.getNonVirtualSize())
+ return AggValueSlot::DoesNotOverlap;
+
+ // The tail padding may contain values we need to preserve.
+ return AggValueSlot::MayOverlap;
+}
+
void CIRGenFunction::emitAggExpr(const Expr *e, AggValueSlot slot) {
AggExprEmitter(*this, slot).Visit(const_cast<Expr *>(e));
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 76353bae68e21..ba854e50861f3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -562,6 +562,19 @@ class CIRGenFunction : public CIRGenTypeCache {
}
Address loadCXXThisAddress();
+ /// Convert the given pointer to a complete class to the given direct base.
+ Address getAddressOfDirectBaseInCompleteClass(mlir::Location loc,
+ Address value,
+ const CXXRecordDecl *derived,
+ const CXXRecordDecl *base,
+ bool baseIsVirtual);
+
+ /// Determine whether a base class initialization may overlap some other
+ /// object.
+ AggValueSlot::Overlap_t getOverlapForBaseInit(const CXXRecordDecl *rd,
+ const CXXRecordDecl *baseRD,
+ bool isVirtual);
+
/// Get an appropriate 'undef' rvalue for the given type.
/// TODO: What's the equivalent for MLIR? Currently we're only using this for
/// void types so it just returns RValue::get(nullptr) but it'll need
@@ -762,6 +775,9 @@ class CIRGenFunction : public CIRGenTypeCache {
void emitAutoVarCleanups(const AutoVarEmission &emission);
void emitAutoVarInit(const AutoVarEmission &emission);
+ void emitBaseInitializer(mlir::Location loc, const CXXRecordDecl *classDecl,
+ CXXCtorInitializer *baseInit);
+
LValue emitBinaryOperatorLValue(const BinaryOperator *e);
mlir::LogicalResult emitBreakStmt(const clang::BreakStmt &s);
diff --git a/clang/test/CIR/CodeGen/ctor.cpp b/clang/test/CIR/CodeGen/ctor.cpp
index 4c2877f8460d0..2b06bb0f7cb08 100644
--- a/clang/test/CIR/CodeGen/ctor.cpp
+++ b/clang/test/CIR/CodeGen/ctor.cpp
@@ -219,3 +219,130 @@ void init_union() {
// CHECK-NEXT: %[[S_ADDR:.*]] = cir.alloca {{.*}} ["s", init]
// CHECK-NEXT: cir.call @_ZN14UnionInitStrukC1Ev(%[[S_ADDR]])
// CHECK-NEXT: cir.return
+
+struct Base {
+ int a;
+ Base(int val) : a(val) {}
+};
+
+struct Derived : Base {
+ Derived(int val) : Base(val) {}
+};
+
+void test_derived() {
+ Derived d(1);
+}
+
+// CHECK: cir.func{{.*}} @_ZN4BaseC2Ei(%arg0: !cir.ptr<!rec_Base> {{.*}}, %arg1: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL_ADDR:.*]] = cir.alloca {{.*}} ["val", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[A_ADDR:.*]] = cir.get_member %[[THIS]][0] {name = "a"}
+// CHECK-NEXT: %[[VAL:.*]] = cir.load{{.*}} %[[VAL_ADDR]]
+// CHECK-NEXT: cir.store{{.*}} %[[VAL]], %[[A_ADDR]]
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_ZN7DerivedC2Ei(%arg0: !cir.ptr<!rec_Derived> {{.*}}, %arg1: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL_ADDR:.*]] = cir.alloca {{.*}} ["val", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Derived> nonnull [0] -> !cir.ptr<!rec_Base>
+// CHECK-NEXT: %[[VAL:.*]] = cir.load{{.*}} %[[VAL_ADDR]]
+// CHECK-NEXT: cir.call @_ZN4BaseC2Ei(%[[BASE]], %[[VAL]])
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_ZN7DerivedC1Ei(%arg0: !cir.ptr<!rec_Derived> {{.*}}, %arg1: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL_ADDR:.*]] = cir.alloca {{.*}} ["val", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[VAL:.*]] = cir.load{{.*}} %[[VAL_ADDR]]
+// CHECK-NEXT: cir.call @_ZN7DerivedC2Ei(%[[THIS]], %[[VAL]])
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_Z12test_derivedv
+// CHECK-NEXT: %[[D_ADDR:.*]] = cir.alloca {{.*}} ["d", init]
+// CHECK-NEXT: %[[ONE:.*]] = cir.const #cir.int<1> : !s32i
+// CHECK-NEXT: cir.call @_ZN7DerivedC1Ei(%[[D_ADDR]], %[[ONE]])
+// CHECK-NEXT: cir.return
+
+struct Base2 {
+ int b;
+ Base2(int val) : b(val) {}
+};
+
+struct Derived2 : Base, Base2 {
+ int c;
+ Derived2(int val1, int val2, int val3) : Base(val1), Base2(val2), c(val3) {}
+};
+
+void test_derived2() {
+ Derived2 d(1, 2, 3);
+}
+
+// CHECK: cir.func{{.*}} @_ZN5Base2C2Ei(%arg0: !cir.ptr<!rec_Base2> {{.*}}, %arg1: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL_ADDR:.*]] = cir.alloca {{.*}} ["val", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[B_ADDR:.*]] = cir.get_member %[[THIS]][0] {name = "b"}
+// CHECK-NEXT: %[[VAL:.*]] = cir.load{{.*}} %[[VAL_ADDR]]
+// CHECK-NEXT: cir.store{{.*}} %[[VAL]], %[[B_ADDR]]
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_ZN8Derived2C2Eiii(%arg0: !cir.ptr<!rec_Derived2>
+// CHECK-SAME: %arg1: !s32i
+// CHECK-SAME: %arg2: !s32i
+// CHECK-SAME: %arg3: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL1_ADDR:.*]] = cir.alloca {{.*}} ["val1", init]
+// CHECK-NEXT: %[[VAL2_ADDR:.*]] = cir.alloca {{.*}} ["val2", init]
+// CHECK-NEXT: %[[VAL3_ADDR:.*]] = cir.alloca {{.*}} ["val3", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL1_ADDR]]
+// CHECK-NEXT: cir.store %arg2, %[[VAL2_ADDR]]
+// CHECK-NEXT: cir.store %arg3, %[[VAL3_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Derived2> nonnull [0] -> !cir.ptr<!rec_Base>
+// CHECK-NEXT: %[[VAL1:.*]] = cir.load{{.*}} %[[VAL1_ADDR]]
+// CHECK-NEXT: cir.call @_ZN4BaseC2Ei(%[[BASE]], %[[VAL1]])
+// CHECK-NEXT: %[[BASE2:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Derived2> nonnull [4] -> !cir.ptr<!rec_Base2>
+// CHECK-NEXT: %[[VAL2:.*]] = cir.load{{.*}} %[[VAL2_ADDR]]
+// CHECK-NEXT: cir.call @_ZN5Base2C2Ei(%[[BASE2]], %[[VAL2]])
+// CHECK-NEXT: %[[C_ADDR:.*]] = cir.get_member %[[THIS]][2] {name = "c"}
+// CHECK-NEXT: %[[VAL3:.*]] = cir.load{{.*}} %[[VAL3_ADDR]]
+// CHECK-NEXT: cir.store{{.*}} %[[VAL3]], %[[C_ADDR]]
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_ZN8Derived2C1Eiii(%arg0: !cir.ptr<!rec_Derived2>
+// CHECK-SAME: %arg1: !s32i
+// CHECK-SAME: %arg2: !s32i
+// CHECK-SAME: %arg3: !s32i
+// CHECK-NEXT: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CHECK-NEXT: %[[VAL1_ADDR:.*]] = cir.alloca {{.*}} ["val1", init]
+// CHECK-NEXT: %[[VAL2_ADDR:.*]] = cir.alloca {{.*}} ["val2", init]
+// CHECK-NEXT: %[[VAL3_ADDR:.*]] = cir.alloca {{.*}} ["val3", init]
+// CHECK-NEXT: cir.store %arg0, %[[THIS_ADDR]]
+// CHECK-NEXT: cir.store %arg1, %[[VAL1_ADDR]]
+// CHECK-NEXT: cir.store %arg2, %[[VAL2_ADDR]]
+// CHECK-NEXT: cir.store %arg3, %[[VAL3_ADDR]]
+// CHECK-NEXT: %[[THIS:.*]] = cir.load{{.*}} %[[THIS_ADDR]]
+// CHECK-NEXT: %[[VAL1:.*]] = cir.load{{.*}} %[[VAL1_ADDR]]
+// CHECK-NEXT: %[[VAL2:.*]] = cir.load{{.*}} %[[VAL2_ADDR]]
+// CHECK-NEXT: %[[VAL3:.*]] = cir.load{{.*}} %[[VAL3_ADDR]]
+// CHECK-NEXT: cir.call @_ZN8Derived2C2Eiii(%[[THIS]], %[[VAL1]], %[[VAL2]], %[[VAL3]])
+// CHECK-NEXT: cir.return
+
+// CHECK: cir.func{{.*}} @_Z13test_derived2v
+// CHECK-NEXT: %[[D_ADDR:.*]] = cir.alloca {{.*}} ["d", init]
+// CHECK-NEXT: %[[ONE:.*]] = cir.const #cir.int<1> : !s32i
+// CHECK-NEXT: %[[TWO:.*]] = cir.const #cir.int<2> : !s32i
+// CHECK-NEXT: %[[THREE:.*]] = cir.const #cir.int<3> : !s32i
+// CHECK-NEXT: cir.call @_ZN8Derived2C1Eiii(%[[D_ADDR]], %[[ONE]], %[[TWO]], %[[THREE]])
+// CHECK-NEXT: cir.return
|
xlauko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This change adds support for initializing non-virtual base classes during the prologue of a derived class' constructor.