-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Handle null base class initialization #167023
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 adds handling for null base class initialization, but only for the trivial case where the class is empty. This also moves emitCXXConstructExpr to CIRGenExprCXX.cpp for consistency with classic codegen and the incubator repo.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis adds handling for null base class initialization, but only for the trivial case where the class is empty. This also moves emitCXXConstructExpr to CIRGenExprCXX.cpp for consistency with classic codegen and the incubator repo. Full diff: https://github.com/llvm/llvm-project/pull/167023.diff 4 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 422fa1cf5ad2e..9bb76894c13f1 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -2134,79 +2134,6 @@ RValue CIRGenFunction::emitCXXMemberCallExpr(const CXXMemberCallExpr *ce,
ce, md, returnValue, hasQualifier, qualifier, isArrow, base);
}
-void CIRGenFunction::emitCXXConstructExpr(const CXXConstructExpr *e,
- AggValueSlot dest) {
- assert(!dest.isIgnored() && "Must have a destination!");
- const CXXConstructorDecl *cd = e->getConstructor();
-
- // If we require zero initialization before (or instead of) calling the
- // constructor, as can be the case with a non-user-provided default
- // constructor, emit the zero initialization now, unless destination is
- // already zeroed.
- if (e->requiresZeroInitialization() && !dest.isZeroed()) {
- switch (e->getConstructionKind()) {
- case CXXConstructionKind::Delegating:
- case CXXConstructionKind::Complete:
- emitNullInitialization(getLoc(e->getSourceRange()), dest.getAddress(),
- e->getType());
- break;
- case CXXConstructionKind::VirtualBase:
- case CXXConstructionKind::NonVirtualBase:
- cgm.errorNYI(e->getSourceRange(),
- "emitCXXConstructExpr: base requires initialization");
- break;
- }
- }
-
- // If this is a call to a trivial default constructor, do nothing.
- if (cd->isTrivial() && cd->isDefaultConstructor())
- return;
-
- // Elide the constructor if we're constructing from a temporary
- if (getLangOpts().ElideConstructors && e->isElidable()) {
- // FIXME: This only handles the simplest case, where the source object is
- // passed directly as the first argument to the constructor. This
- // should also handle stepping through implicit casts and conversion
- // sequences which involve two steps, with a conversion operator
- // follwed by a converting constructor.
- const Expr *srcObj = e->getArg(0);
- assert(srcObj->isTemporaryObject(getContext(), cd->getParent()));
- assert(
- getContext().hasSameUnqualifiedType(e->getType(), srcObj->getType()));
- emitAggExpr(srcObj, dest);
- return;
- }
-
- if (const ArrayType *arrayType = getContext().getAsArrayType(e->getType())) {
- assert(!cir::MissingFeatures::sanitizers());
- emitCXXAggrConstructorCall(cd, arrayType, dest.getAddress(), e, false);
- } else {
-
- clang::CXXCtorType type = Ctor_Complete;
- bool forVirtualBase = false;
- bool delegating = false;
-
- switch (e->getConstructionKind()) {
- case CXXConstructionKind::Complete:
- type = Ctor_Complete;
- break;
- case CXXConstructionKind::Delegating:
- // We should be emitting a constructor; GlobalDecl will assert this
- type = curGD.getCtorType();
- delegating = true;
- break;
- case CXXConstructionKind::VirtualBase:
- forVirtualBase = true;
- [[fallthrough]];
- case CXXConstructionKind::NonVirtualBase:
- type = Ctor_Base;
- break;
- }
-
- emitCXXConstructorCall(cd, type, forVirtualBase, delegating, dest, e);
- }
-}
-
RValue CIRGenFunction::emitReferenceBindingToExpr(const Expr *e) {
// Emit the expression as an lvalue.
LValue lv = emitLValue(e);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
index 201fb73983155..c5f81b1946d5c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
@@ -779,8 +779,8 @@ void AggExprEmitter::visitCXXParenListOrInitListExpr(
Expr *e, ArrayRef<Expr *> args, FieldDecl *initializedFieldInUnion,
Expr *arrayFiller) {
- const AggValueSlot dest =
- ensureSlot(cgf.getLoc(e->getSourceRange()), e->getType());
+ const mlir::Location loc = cgf.getLoc(e->getSourceRange());
+ const AggValueSlot dest = ensureSlot(loc, e->getType());
if (e->getType()->isConstantArrayType()) {
cir::ArrayType arrayTy =
@@ -819,10 +819,23 @@ void AggExprEmitter::visitCXXParenListOrInitListExpr(
if (auto *cxxrd = dyn_cast<CXXRecordDecl>(record)) {
assert(numInitElements >= cxxrd->getNumBases() &&
"missing initializer for base class");
- if (cxxrd->getNumBases() > 0) {
- cgf.cgm.errorNYI(e->getSourceRange(),
- "visitCXXParenListOrInitListExpr base class init");
- return;
+ for (auto &base : cxxrd->bases()) {
+ assert(!base.isVirtual() && "should not see vbases here");
+ CXXRecordDecl *baseRD = base.getType()->getAsCXXRecordDecl();
+ Address address = cgf.getAddressOfDirectBaseInCompleteClass(
+ loc, dest.getAddress(), cxxrd, baseRD,
+ /*isBaseVirtual*/ false);
+ assert(!cir::MissingFeatures::aggValueSlotGC());
+ AggValueSlot aggSlot = AggValueSlot::forAddr(
+ address, Qualifiers(), AggValueSlot::IsDestructed,
+ AggValueSlot::IsNotAliased,
+ cgf.getOverlapForBaseInit(cxxrd, baseRD, false));
+ cgf.emitAggExpr(args[curInitIndex++], aggSlot);
+ if (base.getType().isDestructedType()) {
+ cgf.cgm.errorNYI(e->getSourceRange(),
+ "push deferred deactivation cleanup");
+ return;
+ }
}
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
index 9dd9b6d550763..ac126965a95a5 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
@@ -234,6 +234,89 @@ RValue CIRGenFunction::emitCXXMemberOrOperatorCall(
return emitCall(fnInfo, callee, returnValue, args, nullptr, loc);
}
+static void emitNullBaseClassInitialization(CIRGenFunction &cgf,
+ Address destPtr,
+ const CXXRecordDecl *base) {
+ if (base->isEmpty())
+ return;
+
+ cgf.cgm.errorNYI(base->getSourceRange(),
+ "emitNullBaseClassInitialization: not empty");
+}
+
+void CIRGenFunction::emitCXXConstructExpr(const CXXConstructExpr *e,
+ AggValueSlot dest) {
+ assert(!dest.isIgnored() && "Must have a destination!");
+ const CXXConstructorDecl *cd = e->getConstructor();
+
+ // If we require zero initialization before (or instead of) calling the
+ // constructor, as can be the case with a non-user-provided default
+ // constructor, emit the zero initialization now, unless destination is
+ // already zeroed.
+ if (e->requiresZeroInitialization() && !dest.isZeroed()) {
+ switch (e->getConstructionKind()) {
+ case CXXConstructionKind::Delegating:
+ case CXXConstructionKind::Complete:
+ emitNullInitialization(getLoc(e->getSourceRange()), dest.getAddress(),
+ e->getType());
+ break;
+ case CXXConstructionKind::VirtualBase:
+ case CXXConstructionKind::NonVirtualBase:
+ emitNullBaseClassInitialization(*this, dest.getAddress(),
+ cd->getParent());
+ break;
+ }
+ }
+
+ // If this is a call to a trivial default constructor, do nothing.
+ if (cd->isTrivial() && cd->isDefaultConstructor())
+ return;
+
+ // Elide the constructor if we're constructing from a temporary
+ if (getLangOpts().ElideConstructors && e->isElidable()) {
+ // FIXME: This only handles the simplest case, where the source object is
+ // passed directly as the first argument to the constructor. This
+ // should also handle stepping through implicit casts and conversion
+ // sequences which involve two steps, with a conversion operator
+ // follwed by a converting constructor.
+ const Expr *srcObj = e->getArg(0);
+ assert(srcObj->isTemporaryObject(getContext(), cd->getParent()));
+ assert(
+ getContext().hasSameUnqualifiedType(e->getType(), srcObj->getType()));
+ emitAggExpr(srcObj, dest);
+ return;
+ }
+
+ if (const ArrayType *arrayType = getContext().getAsArrayType(e->getType())) {
+ assert(!cir::MissingFeatures::sanitizers());
+ emitCXXAggrConstructorCall(cd, arrayType, dest.getAddress(), e, false);
+ } else {
+
+ clang::CXXCtorType type = Ctor_Complete;
+ bool forVirtualBase = false;
+ bool delegating = false;
+
+ switch (e->getConstructionKind()) {
+ case CXXConstructionKind::Complete:
+ type = Ctor_Complete;
+ break;
+ case CXXConstructionKind::Delegating:
+ // We should be emitting a constructor; GlobalDecl will assert this
+ type = curGD.getCtorType();
+ delegating = true;
+ break;
+ case CXXConstructionKind::VirtualBase:
+ forVirtualBase = true;
+ [[fallthrough]];
+ case CXXConstructionKind::NonVirtualBase:
+ type = Ctor_Base;
+ break;
+ }
+
+ emitCXXConstructorCall(cd, type, forVirtualBase, delegating, dest, e);
+ }
+}
+
static CharUnits calculateCookiePadding(CIRGenFunction &cgf,
const CXXNewExpr *e) {
if (!e->isArray())
diff --git a/clang/test/CIR/CodeGen/ctor-null-init.cpp b/clang/test/CIR/CodeGen/ctor-null-init.cpp
new file mode 100644
index 0000000000000..4324b329c8b41
--- /dev/null
+++ b/clang/test/CIR/CodeGen/ctor-null-init.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir --check-prefix=CIR %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll --check-prefix=LLVM %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll --check-prefix=OGCG %s
+
+struct A {
+ A() = default;
+ A(int); // This constructor triggers the null base class initialization.
+};
+
+struct B : A {
+};
+
+void test_empty_base_null_init() {
+ B{};
+}
+
+// CIR: cir.func {{.*}} @_Z25test_empty_base_null_initv()
+// CIR-NEXT: %[[B_ADDR:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["agg.tmp.ensured"]
+// CIR-NEXT: %[[A_ADDR:.*]] = cir.base_class_addr %[[B_ADDR]] : !cir.ptr<!rec_B> nonnull [0] -> !cir.ptr<!rec_A>
+
+// LLVM: define{{.*}} @_Z25test_empty_base_null_initv()
+// LLVM-NEXT: %[[B:.*]] = alloca %struct.B
+// LLVM-NEXT: ret void
+
+// OGCG: define{{.*}} @_Z25test_empty_base_null_initv()
+// OGCG-NEXT: entry:
+// OGCG-NEXT: %[[B:.*]] = alloca %struct.B
+// OGCG-NEXT: ret void
|
|
@adams381 I can't add you as a reviewer until you have commit access, but you can still add comments. |
| cgm.errorNYI(e->getSourceRange(), | ||
| "emitCXXConstructExpr: base requires initialization"); |
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.
This is the only part of this function changed in this PR. The function is being moved to CIRGenExprCXX.cpp.
| emitNullBaseClassInitialization(*this, dest.getAddress(), | ||
| cd->getParent()); |
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.
This is the only changed code in this function.
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
| CXXRecordDecl *baseRD = base.getType()->getAsCXXRecordDecl(); | ||
| Address address = cgf.getAddressOfDirectBaseInCompleteClass( | ||
| loc, dest.getAddress(), cxxrd, baseRD, | ||
| /*isBaseVirtual*/ false); |
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.
nit:
| /*isBaseVirtual*/ false); | |
| /*baseIsVirtual=*/false); |
This adds handling for null base class initialization, but only for the trivial case where the class is empty. This also moves emitCXXConstructExpr to CIRGenExprCXX.cpp for consistency with classic codegen and the incubator repo.
This adds handling for null base class initialization, but only for the trivial case where the class is empty. This also moves emitCXXConstructExpr to CIRGenExprCXX.cpp for consistency with classic codegen and the incubator repo.