[CIR] Upstream support for emitting constructors #143639
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
andykaylor
requested review from
bcardosolopes,
mmha,
erichkeane,
lanza and
Lancern
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis change upstreams the code to emit simple constructor defintions. Patch is 24.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143639.diff 12 Files Affected:
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index f89d386378e51..df42bf6c1d7cf 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -81,6 +81,7 @@ struct MissingFeatures {
static bool opFuncCPUAndFeaturesAttributes() { return false; }
static bool opFuncSection() { return false; }
static bool opFuncSetComdat() { return false; }
+ static bool opFuncAttributesForDefinition() { return false; }
// CallOp handling
static bool opCallBuiltinFunc() { return false; }
@@ -225,6 +226,9 @@ struct MissingFeatures {
static bool isMemcpyEquivalentSpecialMember() { return false; }
static bool isTrivialCtorOrDtor() { return false; }
static bool implicitConstructorArgs() { return false; }
+ static bool emitCtorPrologue() { return false; }
+ static bool thunks() { return false; }
+ static bool runCleanupsScope() { return false; }
// Missing types
static bool dataMemberType() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
new file mode 100644
index 0000000000000..51751483d34e9
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
@@ -0,0 +1,40 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This contains code dealing with C++ code generation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CIRGenFunction.h"
+#include "CIRGenModule.h"
+
+#include "clang/AST/GlobalDecl.h"
+#include "clang/CIR/MissingFeatures.h"
+
+using namespace clang;
+using namespace clang::CIRGen;
+
+cir::FuncOp CIRGenModule::codegenCXXStructor(GlobalDecl gd) {
+ const CIRGenFunctionInfo &fnInfo =
+ getTypes().arrangeCXXStructorDeclaration(gd);
+ cir::FuncType funcType = getTypes().getFunctionType(fnInfo);
+ cir::FuncOp fn = getAddrOfCXXStructor(gd, &fnInfo, /*FnType=*/nullptr,
+ /*DontDefer=*/true, ForDefinition);
+ assert(!cir::MissingFeatures::opFuncLinkage());
+ CIRGenFunction cgf{*this, builder};
+ curCGF = &cgf;
+ {
+ mlir::OpBuilder::InsertionGuard guard(builder);
+ cgf.generateCode(gd, fn, funcType);
+ }
+ curCGF = nullptr;
+
+ setNonAliasAttributes(gd, fn);
+ assert(!cir::MissingFeatures::opFuncAttributesForDefinition());
+ return fn;
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h
index 107535ebc7275..7d2b46d50d82e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h
+++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h
@@ -37,6 +37,10 @@ class CIRGenCXXABI {
void setCXXABIThisValue(CIRGenFunction &cgf, mlir::Value thisPtr);
+ /// Emit a single constructor/destructor with the gien type from a C++
+ /// constructor Decl.
+ virtual void emitCXXStructor(clang::GlobalDecl gd) = 0;
+
public:
clang::ImplicitParamDecl *getThisDecl(CIRGenFunction &cgf) {
return cgf.cxxabiThisDecl;
@@ -55,12 +59,29 @@ class CIRGenCXXABI {
return md->getParent();
}
+ /// Return whether the given global decl needs a VTT parameter.
+ virtual bool needsVTTParameter(clang::GlobalDecl gd) { return false; }
+
/// Build a parameter variable suitable for 'this'.
void buildThisParam(CIRGenFunction &cgf, FunctionArgList ¶ms);
/// Loads the incoming C++ this pointer as it was passed by the caller.
mlir::Value loadIncomingCXXThis(CIRGenFunction &cgf);
+ /// Emit constructor variants required by this ABI.
+ virtual void emitCXXConstructors(const clang::CXXConstructorDecl *d) = 0;
+
+ /// Insert any ABI-specific implicit parameters into the parameter list for a
+ /// function. This generally involves extra data for constructors and
+ /// destructors.
+ ///
+ /// ABIs may also choose to override the return type, which has been
+ /// initialized with the type of 'this' if HasThisReturn(CGF.CurGD) is true or
+ /// the formal return type of the function otherwise.
+ virtual void addImplicitStructorParams(CIRGenFunction &cgf,
+ clang::QualType &resTy,
+ FunctionArgList ¶ms) = 0;
+
/// Returns true if the given constructor or destructor is one of the kinds
/// that the ABI says returns 'this' (only applies when called non-virtually
/// for destructors).
diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
index 9d25eea9e413d..da754e0806b2d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
@@ -162,6 +162,47 @@ arrangeCIRFunctionInfo(CIRGenTypes &cgt, SmallVectorImpl<CanQualType> &prefix,
return cgt.arrangeCIRFunctionInfo(resultType, prefix, required);
}
+void CIRGenFunction::emitDelegateCallArg(CallArgList &args,
+ const VarDecl *param,
+ SourceLocation loc) {
+ // StartFunction converted the ABI-lowered parameter(s) into a local alloca.
+ // We need to turn that into an r-value suitable for emitCall
+ Address local = getAddrOfLocalVar(param);
+
+ QualType type = param->getType();
+
+ if (const auto *rd = type->getAsCXXRecordDecl()) {
+ cgm.errorNYI(param->getSourceRange(),
+ "emitDelegateCallArg: record argument");
+ return;
+ }
+
+ // GetAddrOfLocalVar returns a pointer-to-pointer for references, but the
+ // argument needs to be the original pointer.
+ if (type->isReferenceType()) {
+ args.add(
+ RValue::get(builder.createLoad(getLoc(param->getSourceRange()), local)),
+ type);
+ } else if (getLangOpts().ObjCAutoRefCount) {
+ cgm.errorNYI(param->getSourceRange(),
+ "emitDelegateCallArg: ObjCAutoRefCount");
+ // For the most part, we just need to load the alloca, except that aggregate
+ // r-values are actually pointers to temporaries.
+ } else {
+ cgm.errorNYI(param->getSourceRange(),
+ "emitDelegateCallArg: convertTempToRValue");
+ }
+
+ // Deactivate the cleanup for the callee-destructed param that was pushed.
+ assert(!cir::MissingFeatures::thunks());
+ if (type->isRecordType() &&
+ type->castAs<RecordType>()->getDecl()->isParamDestroyedInCallee() &&
+ param->needsDestruction(getContext())) {
+ cgm.errorNYI(param->getSourceRange(),
+ "emitDelegateCallArg: callee-destructed param");
+ }
+}
+
static const CIRGenFunctionInfo &
arrangeFreeFunctionLikeCall(CIRGenTypes &cgt, CIRGenModule &cgm,
const CallArgList &args,
diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
index 8491a66ea6cb4..4590775462009 100644
--- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
@@ -21,6 +21,88 @@
using namespace clang;
using namespace clang::CIRGen;
+/// Checks whether the given constructor is a valid subject for the
+/// complete-to-base constructor delgation optimization, i.e. emitting the
+/// complete constructor as a simple call to the base constructor.
+bool CIRGenFunction::isConstructorDelegationValid(
+ const CXXConstructorDecl *ctor) {
+
+ // Currently we disable the optimization for classes with virtual bases
+ // because (1) the address of parameter variables need to be consistent across
+ // all initializers but (2) the delegate function call necessarily creates a
+ // second copy of the parameter variable.
+ //
+ // The limiting example (purely theoretical AFAIK):
+ // struct A { A(int &c) { c++; } };
+ // struct A : virtual A {
+ // B(int count) : A(count) { printf("%d\n", count); }
+ // };
+ // ...although even this example could in principle be emitted as a delegation
+ // since the address of the parameter doesn't escape.
+ if (ctor->getParent()->getNumVBases())
+ return false;
+
+ // We also disable the optimization for variadic functions because it's
+ // impossible to "re-pass" varargs.
+ if (ctor->getType()->castAs<FunctionProtoType>()->isVariadic())
+ return false;
+
+ // FIXME: Decide if we can do a delegation of a delegating constructor.
+ if (ctor->isDelegatingConstructor())
+ return false;
+
+ return true;
+}
+
+Address CIRGenFunction::loadCXXThisAddress() {
+ assert(curFuncDecl && "loading 'this' without a func declaration?");
+ assert(isa<CXXMethodDecl>(curFuncDecl));
+
+ // Lazily compute CXXThisAlignment.
+ if (cxxThisAlignment.isZero()) {
+ // Just use the best known alignment for the parent.
+ // TODO: if we're currently emitting a complete-object ctor/dtor, we can
+ // always use the complete-object alignment.
+ auto rd = cast<CXXMethodDecl>(curFuncDecl)->getParent();
+ cxxThisAlignment = cgm.getClassPointerAlignment(rd);
+ }
+
+ return Address(loadCXXThis(), cxxThisAlignment);
+}
+
+void CIRGenFunction::emitDelegateCXXConstructorCall(
+ const CXXConstructorDecl *ctor, CXXCtorType ctorType,
+ const FunctionArgList &args, SourceLocation loc) {
+ CallArgList delegateArgs;
+
+ FunctionArgList::const_iterator i = args.begin(), e = args.end();
+ assert(i != e && "no parameters to constructor");
+
+ // this
+ Address thisAddr = loadCXXThisAddress();
+ delegateArgs.add(RValue::get(thisAddr.getPointer()), (*i)->getType());
+ ++i;
+
+ // FIXME: The location of the VTT parameter in the parameter list is specific
+ // to the Itanium ABI and shouldn't be hardcoded here.
+ if (cgm.getCXXABI().needsVTTParameter(curGD)) {
+ cgm.errorNYI(loc, "emitDelegateCXXConstructorCall: VTT parameter");
+ return;
+ }
+
+ // Explicit arguments.
+ for (; i != e; ++i) {
+ const VarDecl *param = *i;
+ // FIXME: per-argument source location
+ emitDelegateCallArg(delegateArgs, param, loc);
+ }
+
+ assert(!cir::MissingFeatures::sanitizers());
+
+ emitCXXConstructorCall(ctor, ctorType, /*ForVirtualBase=*/false,
+ /*Delegating=*/true, thisAddr, delegateArgs, loc);
+}
+
Address CIRGenFunction::getAddressOfBaseClass(
Address value, const CXXRecordDecl *derived,
llvm::iterator_range<CastExpr::path_const_iterator> path,
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index e32a5c836be02..5b2094ff7ceb3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -465,7 +465,7 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
if (isa<CXXDestructorDecl>(funcDecl))
getCIRGenModule().errorNYI(bodyRange, "C++ destructor definition");
else if (isa<CXXConstructorDecl>(funcDecl))
- getCIRGenModule().errorNYI(bodyRange, "C++ constructor definition");
+ emitConstructorBody(args);
else if (getLangOpts().CUDA && !getLangOpts().CUDAIsDevice &&
funcDecl->hasAttr<CUDAGlobalAttr>())
getCIRGenModule().errorNYI(bodyRange, "CUDA kernel");
@@ -496,6 +496,47 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
return fn;
}
+void CIRGenFunction::emitConstructorBody(FunctionArgList &args) {
+ assert(!cir::MissingFeatures::sanitizers());
+ const auto *ctor = cast<CXXConstructorDecl>(curGD.getDecl());
+ CXXCtorType ctorType = curGD.getCtorType();
+
+ assert((cgm.getTarget().getCXXABI().hasConstructorVariants() ||
+ ctorType == Ctor_Complete) &&
+ "can only generate complete ctor for this ABI");
+
+ if (ctorType == Ctor_Complete && isConstructorDelegationValid(ctor) &&
+ cgm.getTarget().getCXXABI().hasConstructorVariants()) {
+ emitDelegateCXXConstructorCall(ctor, Ctor_Base, args, ctor->getEndLoc());
+ return;
+ }
+
+ const FunctionDecl *definition = nullptr;
+ Stmt *body = ctor->getBody(definition);
+ assert(definition == ctor && "emitting wrong constructor body");
+
+ if (isa_and_nonnull<CXXTryStmt>(body)) {
+ cgm.errorNYI(ctor->getSourceRange(), "emitConstructorBody: try body");
+ return;
+ }
+
+ assert(!cir::MissingFeatures::incrementProfileCounter());
+ assert(!cir::MissingFeatures::runCleanupsScope());
+
+ // TODO: in restricted cases, we can emit the vbase initializers of a
+ // complete ctor and then delegate to the base ctor.
+
+ assert(!cir::MissingFeatures::emitCtorPrologue());
+
+ // TODO(cir): propagate this result via mlir::logical result. Just unreachable
+ // now just to have it handled.
+ if (mlir::failed(emitStmt(body, true))) {
+ cgm.errorNYI(ctor->getSourceRange(),
+ "emitConstructorBody: emit body statement failed.");
+ return;
+ }
+}
+
/// Given a value of type T* that may not be to a complete object, construct
/// an l-vlaue withi the natural pointee alignment of T.
LValue CIRGenFunction::makeNaturalAlignPointeeAddrLValue(mlir::Value val,
@@ -522,16 +563,16 @@ clang::QualType CIRGenFunction::buildFunctionArgList(clang::GlobalDecl gd,
cgm.getCXXABI().buildThisParam(*this, args);
}
- if (isa<CXXConstructorDecl>(fd))
- cgm.errorNYI(fd->getSourceRange(),
- "buildFunctionArgList: CXXConstructorDecl");
+ if (const auto *cd = dyn_cast<CXXConstructorDecl>(fd))
+ if (cd->getInheritedConstructor())
+ cgm.errorNYI(fd->getSourceRange(),
+ "buildFunctionArgList: inherited constructor");
for (auto *param : fd->parameters())
args.push_back(param);
if (md && (isa<CXXConstructorDecl>(md) || isa<CXXDestructorDecl>(md)))
- cgm.errorNYI(fd->getSourceRange(),
- "buildFunctionArgList: implicit structor params");
+ cgm.getCXXABI().addImplicitStructorParams(*this, retTy, args);
return retTy;
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 7db7f6928fd8f..c45ab8ee66e87 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -60,6 +60,7 @@ class CIRGenFunction : public CIRGenTypeCache {
ImplicitParamDecl *cxxabiThisDecl = nullptr;
mlir::Value cxxabiThisValue = nullptr;
mlir::Value cxxThisValue = nullptr;
+ clang::CharUnits cxxThisAlignment;
// Holds the Decl for the current outermost non-closure context
const clang::Decl *curFuncDecl = nullptr;
@@ -467,6 +468,9 @@ class CIRGenFunction : public CIRGenTypeCache {
bool shouldNullCheckClassCastValue(const CastExpr *ce);
+ static bool
+ isConstructorDelegationValid(const clang::CXXConstructorDecl *ctor);
+
LValue makeNaturalAlignPointeeAddrLValue(mlir::Value v, clang::QualType t);
/// Construct an address with the natural alignment of T. If a pointer to T
@@ -511,6 +515,7 @@ class CIRGenFunction : public CIRGenTypeCache {
assert(cxxThisValue && "no 'this' value for this function");
return cxxThisValue;
}
+ Address loadCXXThisAddress();
/// Get an appropriate 'undef' rvalue for the given type.
/// TODO: What's the equivalent for MLIR? Currently we're only using this for
@@ -742,6 +747,8 @@ class CIRGenFunction : public CIRGenTypeCache {
LValue emitCompoundAssignmentLValue(const clang::CompoundAssignOperator *e);
+ void emitConstructorBody(FunctionArgList &args);
+
mlir::LogicalResult emitContinueStmt(const clang::ContinueStmt &s);
void emitCXXConstructExpr(const clang::CXXConstructExpr *e,
@@ -830,6 +837,17 @@ class CIRGenFunction : public CIRGenTypeCache {
mlir::Type condType,
bool buildingTopLevelCase);
+ void emitDelegateCXXConstructorCall(const clang::CXXConstructorDecl *ctor,
+ clang::CXXCtorType ctorType,
+ const FunctionArgList &args,
+ clang::SourceLocation loc);
+
+ /// We are performing a delegate call; that is, the current function is
+ /// delegating to another one. Produce a r-value suitable for passing the
+ /// given parameter.
+ void emitDelegateCallArg(CallArgList &args, const clang::VarDecl *param,
+ clang::SourceLocation loc);
+
/// Emit an `if` on a boolean condition to the specified blocks.
/// FIXME: Based on the condition, this might try to simplify the codegen of
/// the conditional based on the branch.
diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
index fdd8b63fb6da0..39b03a12ede9a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
@@ -20,7 +20,9 @@
#include "CIRGenCXXABI.h"
#include "CIRGenFunction.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/GlobalDecl.h"
+#include "clang/CIR/MissingFeatures.h"
#include "llvm/Support/ErrorHandling.h"
using namespace clang;
@@ -35,8 +37,16 @@ class CIRGenItaniumCXXABI : public CIRGenCXXABI {
assert(!cir::MissingFeatures::cxxabiUseARMGuardVarABI());
}
- void emitInstanceFunctionProlog(SourceLocation Loc,
- CIRGenFunction &CGF) override;
+ bool needsVTTParameter(clang::GlobalDecl gd) override;
+
+ void emitInstanceFunctionProlog(SourceLocation loc,
+ CIRGenFunction &cgf) override;
+
+ void addImplicitStructorParams(CIRGenFunction &cgf, QualType &retTy,
+ FunctionArgList ¶ms) override;
+
+ void emitCXXConstructors(const clang::CXXConstructorDecl *d) override;
+ void emitCXXStructor(clang::GlobalDecl gd) override;
};
} // namespace
@@ -72,6 +82,72 @@ void CIRGenItaniumCXXABI::emitInstanceFunctionProlog(SourceLocation loc,
}
}
+void CIRGenItaniumCXXABI::addImplicitStructorParams(CIRGenFunction &cgf,
+ QualType &retTY,
+ FunctionArgList ¶ms) {
+ const auto *md = cast<CXXMethodDecl>(cgf.curGD.getDecl());
+ assert(isa<CXXConstructorDecl>(md) || isa<CXXDestructorDecl>(md));
+
+ // Check if we need a VTT parameter as well.
+ if (needsVTTParameter(cgf.curGD)) {
+ cgf.cgm.errorNYI(cgf.curFuncDecl->getLocation(),
+ "addImplicitStructorParams: VTT parameter");
+ }
+}
+
+void CIRGenItaniumCXXABI::emitCXXStructor(GlobalDecl gd) {
+ auto *md = cast<CXXMethodDecl>(gd.getDecl());
+ auto *cd = dyn_cast<CXXConstructorDecl>(md);
+
+ if (!cd) {
+ cgm.errorNYI(md->getSourceRange(), "CXCABI emit destructor");
+ return;
+ }
+
+ if (cgm.getCodeGenOpts().CXXCtorDtorAliases)
+ cgm.errorNYI(md->getSourceRange(), "Ctor/Dtor aliases");
+
+ auto fn = cgm.codegenCXXStructor(gd);
+
+ cgm.maybeSetTrivialComdat(*md, fn);
+}
+
+void CIRGenItaniumCXXABI::emitCXXConstructors(const CXXConstructorDecl *d) {
+ // Just make sure we're in sync with TargetCXXABI.
+ assert(cgm.getTarget().getCXXABI().hasConstructorVariants());
+
+ // The constructor used for constructing this as a base class;
+ // ignores virtual bases.
+ cgm.emitGlobal(GlobalDecl(d, Ctor_Base));
+
+ // The constructor used for constructing this as a complete class;
+ // constructs the virtual bases, then calls the base constructor.
+ if (!d->getParent()->isAbstract()) {
+ // We don't need to emit the complete ctro if the class is abstract.
+ cgm.emitGlobal(GlobalDecl(d, Ctor_Complete));
+ }
+}
+
+/// Return whether the given global decl needs a VTT parameter, which it does if
+/// it's a base constructor or destructor with virtual bases.
+bool CIRGenItaniumCXXABI::needsVTTParameter(GlobalDecl gd) {
+ auto *md = cast<CXXMethodDecl>(gd.getDecl());
+
+ // We don't have any virtual bases, just return early.
+ if (!md->getParent()->getNumVBases())
+ return false;
+
+ // Check if we have a base constructor.
+ if (isa<CXXConstructorDecl>(md) && gd.getCtorType() == Ctor_Base)
+ return true;
+
+ // Check if we have a base destructor.
+ if (isa<CXXDestructorDecl>(md) && gd.getDtorType() == Dtor_Base)
+ return true;
+
+ return false;
+}
+
CIRGenCXXABI *clang::CIRGen::CreateCIRGenItaniumCXXABI(CIRGenModule &cgm) {
switch (cgm.getASTContext().getCXXABIKind()) {
case TargetCXXABI::GenericItanium:
diff --git a/clang/lib/CIR/CodeGen/CIRGenModu...
[truncated]
|
Lancern
requested changes
Jun 11, 2025
Comment on lines
24
to
26
| /// Checks whether the given constructor is a valid subject for the | ||
| /// complete-to-base constructor delgation optimization, i.e. emitting the | ||
| /// complete constructor as a simple call to the base constructor. |
There was a problem hiding this comment.
This looks like an optimization to me. Later maybe we should skip this during CIRGen and delay the optimization to later CIR passes?
There was a problem hiding this comment.
Is this sort of optimization something we can do in CIR? This lives IIRC in classic-codegen because LLVM-IR can't represent enough info for this.
There was a problem hiding this comment.
It's not clear to me what the "optimization" is. The code I'm adding in this PR emits the C1 constructor as a call to the C2 constructor. When we upstream support for constructor aliases, there will be an option to omit the C1 constructor and call the C2 constructor directly (which seems like an optimization). Experimenting with the incubator and classic codegen, I find that even when I use -mno-constructor-aliases when this function returns false, the C1 constructor is omitted.
https://godbolt.org/z/5obc7fbnW
Given the current state of the upstream CIR implementation, we can't handle code with virtual bases. I will try a variadic constructor to see what happens. The relevant case for what I've got implemented in this PR is the ctor->isDelegatingConstructor() case (which seems to have the opposite meaning of what I would expect).
In theory, if this function returns false we should emit the full constructor body for both C1 and C2 constructors. In practice, the incubator and classic codegen don't seem to do that.
There was a problem hiding this comment.
Update: I was misunderstanding what isDelegatingConstructor() constructor meant. It's not the relevant case here. This function is only called for the C1 constructor (ctorType == Ctor_Complete) and it returns true for the initial test case here, causing us to emit the constructor as a call to the C2 constructor.
When I add a variadic constructor (which I will in my next update to this PR), this function returns false and we emit the C1 constructor (but not the C2 constructor).
There was a problem hiding this comment.
I don't think there's a good case for doing this optimization later. It would basically require the optimizer to recognize that two constructors were identical and replace the body of one with a call to the other. We could (and maybe should) indicate in some way other than the name mangling which constructor it is we're emitting, and I guess theoretically we could set a flag indicating that the implementations are identical or provide some other means for an optimizer to deduce exactly what we're deciding here. I just don't see any benefit to that versus handling it here, particularly once we have added the support for constructor aliases.
In cases where the base constructor and complete object constructor are identical, we will only emit the base object constructor when constructor aliases are enabled. When constructor aliases are not enabled, we will emit both even in cases where only the complete object constructor is needed, but that will be trivially inlined, which is a much easier problem for the optimizer to handle.
There was a problem hiding this comment.
I think I misunderstood what "delegation optimization" means here. I confused it with "delegating constructors", which made me believe that we already have some information in the generated un-optimized code that could enable this optimization.
I agree that it seems no benefits to delay this optimization to later passes. Actually after playing around for some while in godbolt I find it hard to actually trigger this optimization even with OGCG, because nowadays clang just emits C1 as an alias to C2 by default in case they are identical, which leaves no room for this optimization.
There was a problem hiding this comment.
To see this happen from Compiler Explorer, you need to pass -Xclang -mno-constructor-aliases. Otherwise, we end up just not emitting the C1 constructor at all in cases where it would call the C2 constructor.
erichkeane
approved these changes
Jun 11, 2025
clang/lib/CIR/CodeGen/CIRGenCXXABI.h
Outdated
| @@ -37,6 +37,10 @@ class CIRGenCXXABI { | |||
|
|
|||
| void setCXXABIThisValue(CIRGenFunction &cgf, mlir::Value thisPtr); | |||
|
|
|||
| /// Emit a single constructor/destructor with the gien type from a C++ | |||
There was a problem hiding this comment.
Suggested change
| /// Emit a single constructor/destructor with the gien type from a C++ | |
| /// Emit a single constructor/destructor with the gen type from a C++ |
clang/lib/CIR/CodeGen/CIRGenCXXABI.h
Outdated
| @@ -37,6 +37,10 @@ class CIRGenCXXABI { | |||
|
|
|||
| void setCXXABIThisValue(CIRGenFunction &cgf, mlir::Value thisPtr); | |||
|
|
|||
| /// Emit a single constructor/destructor with the gien type from a C++ | |||
| /// constructor Decl. | |||
There was a problem hiding this comment.
Suggested change
| /// constructor Decl. | |
| /// constructor/destructor Decl. |
clang/lib/CIR/CodeGen/CIRGenCXXABI.h
Outdated
| @@ -55,12 +59,29 @@ class CIRGenCXXABI { | |||
| return md->getParent(); | |||
| } | |||
|
|
|||
| /// Return whether the given global decl needs a VTT parameter. | |||
There was a problem hiding this comment.
can we say what a VTT parameter is?
There was a problem hiding this comment.
It's going to look like a typo, but OK. ;-)
| @@ -21,6 +21,88 @@ | |||
| using namespace clang; | |||
| using namespace clang::CIRGen; | |||
|
|
|||
| /// Checks whether the given constructor is a valid subject for the | |||
| /// complete-to-base constructor delgation optimization, i.e. emitting the | |||
There was a problem hiding this comment.
Suggested change
| /// complete-to-base constructor delgation optimization, i.e. emitting the | |
| /// complete-to-base constructor delegation optimization, i.e. emitting the |
Comment on lines
24
to
26
| /// Checks whether the given constructor is a valid subject for the | ||
| /// complete-to-base constructor delgation optimization, i.e. emitting the | ||
| /// complete constructor as a simple call to the base constructor. |
There was a problem hiding this comment.
Is this sort of optimization something we can do in CIR? This lives IIRC in classic-codegen because LLVM-IR can't represent enough info for this.
| const auto *md = cast<CXXMethodDecl>(cgf.curGD.getDecl()); | ||
| assert(isa<CXXConstructorDecl>(md) || isa<CXXDestructorDecl>(md)); | ||
|
|
||
| // Check if we need a VTT parameter as well. |
There was a problem hiding this comment.
as well as what? Seems this function is a noop besides an assert... We probably should find some use of md here else this is going to warn.
There was a problem hiding this comment.
I believe it means "in addition to the params that were passed in. I should probably just remove this function until we're ready to implement the VTT part.
This change upstreams the code to emit simple constructor defintions.
erichkeane
approved these changes
Jun 11, 2025
andykaylor
force-pushed
the
cir-emit-constructor
branch
from
6ba60aa to
55e109a
Compare
Lancern
approved these changes
Jun 12, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/53/builds/16839 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/20/builds/11761 Here is the relevant piece of the build log for the reference |
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
This change upstreams the code to emit simple constructor defintions.
akuhlens
pushed a commit
to akuhlens/llvm-project
that referenced
this pull request
Jun 24, 2025
This change upstreams the code to emit simple constructor defintions.
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 change upstreams the code to emit simple constructor defintions.