[CIR] Upstream support for calling constructors

[andykaylor]

This change adds support for calling C++ constructors. The support for actually defining a constructor is still missing and will be added in a later change.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Andy Kaylor (andykaylor)

Changes

This change adds support for calling C++ constructors. The support for actually defining a constructor is still missing and will be added in a later change.

---

Patch is 22.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143579.diff

10 Files Affected:

• (modified) clang/include/clang/CIR/MissingFeatures.h (+3)
• (modified) clang/lib/CIR/CodeGen/CIRGenCall.cpp (+93-6)
• (modified) clang/lib/CIR/CodeGen/CIRGenClass.cpp (+74)
• (modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+51)
• (modified) clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp (+6)
• (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+13)
• (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+52-2)
• (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+19)
• (modified) clang/lib/CIR/CodeGen/CIRGenTypes.h (+6)
• (added) clang/test/CIR/CodeGen/ctor.cpp (+19)

diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 72d882beb2244..f89d386378e51 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -222,6 +222,9 @@ struct MissingFeatures {
   static bool instrumentation() { return false; }
   static bool cleanupAfterErrorDiags() { return false; }
   static bool cxxRecordStaticMembers() { return false; }
+  static bool isMemcpyEquivalentSpecialMember() { return false; }
+  static bool isTrivialCtorOrDtor() { return false; }
+  static bool implicitConstructorArgs() { return false; }
 
   // Missing types
   static bool dataMemberType() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
index b194a8670bfb9..9d25eea9e413d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
@@ -60,6 +60,13 @@ CIRGenCallee CIRGenCallee::prepareConcreteCallee(CIRGenFunction &cgf) const {
   return *this;
 }
 
+/// Returns the canonical formal type of the given C++ method.
+static CanQual<FunctionProtoType> getFormalType(const CXXMethodDecl *md) {
+  return md->getType()
+      ->getCanonicalTypeUnqualified()
+      .getAs<FunctionProtoType>();
+}
+
 /// Adds the formal parameters in FPT to the given prefix. If any parameter in
 /// FPT has pass_object_size_attrs, then we'll add parameters for those, too.
 /// TODO(cir): this should be shared with LLVM codegen
@@ -76,6 +83,48 @@ static void appendParameterTypes(const CIRGenTypes &cgt,
   cgt.getCGModule().errorNYI("appendParameterTypes: hasExtParameterInfos");
 }
 
+const CIRGenFunctionInfo &
+CIRGenTypes::arrangeCXXStructorDeclaration(GlobalDecl gd) {
+  auto *md = cast<CXXMethodDecl>(gd.getDecl());
+
+  llvm::SmallVector<CanQualType, 16> argTypes;
+  argTypes.push_back(deriveThisType(md->getParent(), md));
+
+  bool passParams = true;
+
+  if (auto *cd = dyn_cast<CXXConstructorDecl>(md)) {
+    // A base class inheriting constructor doesn't get forwarded arguments
+    // needed to construct a virtual base (or base class thereof)
+    if (cd->getInheritedConstructor())
+      cgm.errorNYI(cd->getSourceRange(),
+                   "arrangeCXXStructorDeclaration: inheriting constructor");
+  }
+
+  CanQual<FunctionProtoType> fpt = getFormalType(md);
+
+  if (passParams)
+    appendParameterTypes(*this, argTypes, fpt);
+
+  assert(!cir::MissingFeatures::implicitConstructorArgs());
+
+  RequiredArgs required =
+      (passParams && md->isVariadic() ? RequiredArgs(argTypes.size())
+                                      : RequiredArgs::All);
+
+  CanQualType resultType = theCXXABI.hasThisReturn(gd) ? argTypes.front()
+                           : theCXXABI.hasMostDerivedReturn(gd)
+                               ? astContext.VoidPtrTy
+                               : astContext.VoidTy;
+
+  assert(!theCXXABI.hasThisReturn(gd) &&
+         "Please send PR with a test and remove this");
+
+  assert(!cir::MissingFeatures::opCallCIRGenFuncInfoExtParamInfo());
+  assert(!cir::MissingFeatures::opCallFnInfoOpts());
+
+  return arrangeCIRFunctionInfo(resultType, argTypes, required);
+}
+
 /// Derives the 'this' type for CIRGen purposes, i.e. ignoring method CVR
 /// qualification. Either or both of `rd` and `md` may be null. A null `rd`
 /// indicates that there is no meaningful 'this' type, and a null `md` can occur
@@ -103,13 +152,13 @@ CanQualType CIRGenTypes::deriveThisType(const CXXRecordDecl *rd,
 /// top of any implicit parameters already stored.
 static const CIRGenFunctionInfo &
 arrangeCIRFunctionInfo(CIRGenTypes &cgt, SmallVectorImpl<CanQualType> &prefix,
-                       CanQual<FunctionProtoType> ftp) {
+                       CanQual<FunctionProtoType> fpt) {
   assert(!cir::MissingFeatures::opCallFnInfoOpts());
   RequiredArgs required =
-      RequiredArgs::getFromProtoWithExtraSlots(ftp, prefix.size());
+      RequiredArgs::getFromProtoWithExtraSlots(fpt, prefix.size());
   assert(!cir::MissingFeatures::opCallExtParameterInfo());
-  appendParameterTypes(cgt, prefix, ftp);
-  CanQualType resultType = ftp->getReturnType().getUnqualifiedType();
+  appendParameterTypes(cgt, prefix, fpt);
+  CanQualType resultType = fpt->getReturnType().getUnqualifiedType();
   return cgt.arrangeCIRFunctionInfo(resultType, prefix, required);
 }
 
@@ -141,6 +190,44 @@ arrangeFreeFunctionLikeCall(CIRGenTypes &cgt, CIRGenModule &cgm,
   return cgt.arrangeCIRFunctionInfo(retType, argTypes, required);
 }
 
+/// Arrange a call to a C++ method, passing the given arguments.
+///
+/// passProtoArgs indicates whether `args` has args for the parameters in the
+/// given CXXConstructorDecl.
+const CIRGenFunctionInfo &CIRGenTypes::arrangeCXXConstructorCall(
+    const CallArgList &args, const CXXConstructorDecl *d, CXXCtorType ctorKind,
+    bool passProtoArgs) {
+
+  // FIXME: Kill copy.
+  llvm::SmallVector<CanQualType, 16> argTypes;
+  for (const auto &arg : args)
+    argTypes.push_back(astContext.getCanonicalParamType(arg.ty));
+
+  assert(!cir::MissingFeatures::implicitConstructorArgs());
+  // +1 for implicit this, which should always be args[0]
+  unsigned totalPrefixArgs = 1;
+
+  CanQual<FunctionProtoType> fpt = getFormalType(d);
+  RequiredArgs required =
+      passProtoArgs
+          ? RequiredArgs::getFromProtoWithExtraSlots(fpt, totalPrefixArgs)
+          : RequiredArgs::All;
+
+  GlobalDecl gd(d, ctorKind);
+  if (theCXXABI.hasThisReturn(gd))
+    cgm.errorNYI(d->getSourceRange(),
+                 "arrangeCXXConstructorCall: hasThisReturn");
+  if (theCXXABI.hasMostDerivedReturn(gd))
+    cgm.errorNYI(d->getSourceRange(),
+                 "arrangeCXXConstructorCall: hasMostDerivedReturn");
+  CanQualType resultType = astContext.VoidTy;
+
+  assert(!cir::MissingFeatures::opCallFnInfoOpts());
+  assert(!cir::MissingFeatures::opCallCIRGenFuncInfoExtParamInfo());
+
+  return arrangeCIRFunctionInfo(resultType, argTypes, required);
+}
+
 /// Arrange a call to a C++ method, passing the given arguments.
 ///
 /// numPrefixArgs is the number of the ABI-specific prefix arguments we have. It
@@ -198,7 +285,7 @@ CIRGenTypes::arrangeCXXMethodDeclaration(const CXXMethodDecl *md) {
 /// constructor or destructor.
 const CIRGenFunctionInfo &
 CIRGenTypes::arrangeCXXMethodType(const CXXRecordDecl *rd,
-                                  const FunctionProtoType *ftp,
+                                  const FunctionProtoType *fpt,
                                   const CXXMethodDecl *md) {
   llvm::SmallVector<CanQualType, 16> argTypes;
 
@@ -208,7 +295,7 @@ CIRGenTypes::arrangeCXXMethodType(const CXXRecordDecl *rd,
   assert(!cir::MissingFeatures::opCallFnInfoOpts());
   return ::arrangeCIRFunctionInfo(
       *this, argTypes,
-      ftp->getCanonicalTypeUnqualified().getAs<FunctionProtoType>());
+      fpt->getCanonicalTypeUnqualified().getAs<FunctionProtoType>());
 }
 
 /// Arrange the argument and result information for the declaration or
diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
index 4cdaa480121dd..9492f90b7716c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
@@ -10,9 +10,12 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "CIRGenCXXABI.h"
 #include "CIRGenFunction.h"
 
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/AST/Type.h"
 #include "clang/CIR/MissingFeatures.h"
 
 using namespace clang;
@@ -63,3 +66,74 @@ Address CIRGenFunction::getAddressOfBaseClass(
 
   return value;
 }
+
+void CIRGenFunction::emitCXXConstructorCall(const clang::CXXConstructorDecl *d,
+                                            clang::CXXCtorType type,
+                                            bool forVirtualBase,
+                                            bool delegating,
+                                            AggValueSlot thisAVS,
+                                            const clang::CXXConstructExpr *e) {
+  CallArgList args;
+  Address thisAddr = thisAVS.getAddress();
+  QualType thisType = d->getThisType();
+  mlir::Value thisPtr = thisAddr.getPointer();
+
+  assert(!cir::MissingFeatures::addressSpace());
+
+  args.add(RValue::get(thisPtr), thisType);
+
+  // In LLVM Codegen: If this is a trivial constructor, just emit what's needed.
+  // If this is a union copy constructor, we must emit a memcpy, because the AST
+  // does not model that copy.
+  assert(!cir::MissingFeatures::isMemcpyEquivalentSpecialMember());
+
+  const FunctionProtoType *fpt = d->getType()->castAs<FunctionProtoType>();
+
+  assert(!cir::MissingFeatures::opCallArgEvaluationOrder());
+
+  emitCallArgs(args, fpt, e->arguments(), e->getConstructor(),
+               /*ParamsToSkip=*/0);
+
+  assert(!cir::MissingFeatures::sanitizers());
+  emitCXXConstructorCall(d, type, forVirtualBase, delegating, thisAddr, args,
+                         e->getExprLoc());
+}
+
+void CIRGenFunction::emitCXXConstructorCall(
+    const CXXConstructorDecl *d, CXXCtorType type, bool forVirtualBase,
+    bool delegating, Address thisAddr, CallArgList &args, SourceLocation loc) {
+
+  const auto *cd = d->getParent();
+
+  // If this is a call to a trivial default constructor:
+  // In LLVM: do nothing.
+  // In CIR: emit as a regular call, other later passes should lower the
+  // ctor call into trivial initialization.
+  assert(!cir::MissingFeatures::isTrivialCtorOrDtor());
+
+  assert(!cir::MissingFeatures::isMemcpyEquivalentSpecialMember());
+
+  bool passPrototypeArgs = true;
+
+  // Check whether we can actually emit the constructor before trying to do so.
+  if (d->getInheritedConstructor()) {
+    cgm.errorNYI(d->getSourceRange(),
+                 "emitCXXConstructorCall: inherited constructor");
+    return;
+  }
+
+  // Insert any ABI-specific implicit constructor arguments.
+  assert(!cir::MissingFeatures::implicitConstructorArgs());
+
+  // Emit the call.
+  auto calleePtr = cgm.getAddrOfCXXStructor(GlobalDecl(d, type));
+  const CIRGenFunctionInfo &info = cgm.getTypes().arrangeCXXConstructorCall(
+      args, d, type, passPrototypeArgs);
+  CIRGenCallee callee = CIRGenCallee::forDirect(calleePtr, GlobalDecl(d, type));
+  cir::CIRCallOpInterface c;
+  emitCall(info, callee, ReturnValueSlot(), args, &c, getLoc(loc));
+
+  if (cgm.getCodeGenOpts().OptimizationLevel != 0 && !cd->isDynamicClass() &&
+      type != Ctor_Base && cgm.getCodeGenOpts().StrictVTablePointers)
+    cgm.errorNYI(d->getSourceRange(), "vtable assumption loads");
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 8129fe0ad7db7..f4e0e9e1e2e3d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -1393,6 +1393,57 @@ 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()) {
+    cgm.errorNYI(e->getSourceRange(),
+                 "emitCXXConstructExpr: requires initialization");
+    return;
+  }
+
+  // If this is a call to a trivial default constructor:
+  // In LLVM: do nothing.
+  // In CIR: emit as a regular call, other later passes should lower the
+  // ctor call into trivial initialization.
+
+  // Elide the constructor if we're constructing from a temporary
+  if (getLangOpts().ElideConstructors && e->isElidable()) {
+    cgm.errorNYI(e->getSourceRange(),
+                 "emitCXXConstructExpr: elidable constructor");
+    return;
+  }
+
+  if (const ArrayType *arrayType = getContext().getAsArrayType(e->getType())) {
+    cgm.errorNYI(e->getSourceRange(), "emitCXXConstructExpr: array type");
+    return;
+  } 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:
+    case CXXConstructionKind::VirtualBase:
+    case CXXConstructionKind::NonVirtualBase:
+      cgm.errorNYI(e->getSourceRange(),
+                   "emitCXXConstructExpr: other construction kind");
+      return;
+    }
+
+    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 56d7ea3884ba7..f1df1b79fc48e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
@@ -51,6 +51,7 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
   void Visit(Expr *e) { StmtVisitor<AggExprEmitter>::Visit(e); }
 
   void VisitInitListExpr(InitListExpr *e);
+  void VisitCXXConstructExpr(const CXXConstructExpr *e);
 
   void visitCXXParenListOrInitListExpr(Expr *e, ArrayRef<Expr *> args,
                                        FieldDecl *initializedFieldInUnion,
@@ -213,6 +214,11 @@ void AggExprEmitter::emitInitializationToLValue(Expr *e, LValue lv) {
   }
 }
 
+void AggExprEmitter::VisitCXXConstructExpr(const CXXConstructExpr *e) {
+  AggValueSlot slot = ensureSlot(cgf.getLoc(e->getSourceRange()), e->getType());
+  cgf.emitCXXConstructExpr(e, slot);
+}
+
 void AggExprEmitter::emitNullInitializationToLValue(mlir::Location loc,
                                                     LValue lv) {
   const QualType type = lv.getType();
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index d6002c3e4d4d9..7db7f6928fd8f 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -744,6 +744,19 @@ class CIRGenFunction : public CIRGenTypeCache {
 
   mlir::LogicalResult emitContinueStmt(const clang::ContinueStmt &s);
 
+  void emitCXXConstructExpr(const clang::CXXConstructExpr *e,
+                            AggValueSlot dest);
+
+  void emitCXXConstructorCall(const clang::CXXConstructorDecl *d,
+                              clang::CXXCtorType type, bool forVirtualBase,
+                              bool delegating, AggValueSlot thisAVS,
+                              const clang::CXXConstructExpr *e);
+
+  void emitCXXConstructorCall(const clang::CXXConstructorDecl *d,
+                              clang::CXXCtorType type, bool forVirtualBase,
+                              bool delegating, Address thisAddr,
+                              CallArgList &args, clang::SourceLocation loc);
+
   mlir::LogicalResult emitCXXForRangeStmt(const CXXForRangeStmt &s,
                                           llvm::ArrayRef<const Attr *> attrs);
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 3d46c44b4f1ec..8407f8fad06ba 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -103,6 +103,25 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &mlirContext,
 
 CIRGenModule::~CIRGenModule() = default;
 
+/// FIXME: this could likely be a common helper and not necessarily related
+/// with codegen.
+/// Return the best known alignment for an unknown pointer to a
+/// particular class.
+CharUnits CIRGenModule::getClassPointerAlignment(const CXXRecordDecl *rd) {
+  if (!rd->hasDefinition())
+    return CharUnits::One(); // Hopefully won't be used anywhere.
+
+  auto &layout = astContext.getASTRecordLayout(rd);
+
+  // If the class is final, then we know that the pointer points to an
+  // object of that type and can use the full alignment.
+  if (rd->isEffectivelyFinal())
+    return layout.getAlignment();
+
+  // Otherwise, we have to assume it could be a subclass.
+  return layout.getNonVirtualAlignment();
+}
+
 CharUnits CIRGenModule::getNaturalTypeAlignment(QualType t,
                                                 LValueBaseInfo *baseInfo) {
   assert(!cir::MissingFeatures::opTBAA());
@@ -1174,6 +1193,34 @@ void CIRGenModule::setInitializer(cir::GlobalOp &op, mlir::Attribute value) {
   assert(!cir::MissingFeatures::opGlobalVisibility());
 }
 
+std::pair<cir::FuncType, cir::FuncOp> CIRGenModule::getAddrAndTypeOfCXXStructor(
+    GlobalDecl gd, const CIRGenFunctionInfo *fnInfo, cir::FuncType fnType,
+    bool dontDefer, ForDefinition_t isForDefinition) {
+  auto *md = cast<CXXMethodDecl>(gd.getDecl());
+
+  if (isa<CXXDestructorDecl>(md)) {
+    // Always alias equivalent complete destructors to base destructors in the
+    // MS ABI.
+    if (getTarget().getCXXABI().isMicrosoft() &&
+        gd.getDtorType() == Dtor_Complete &&
+        md->getParent()->getNumVBases() == 0)
+      errorNYI(md->getSourceRange(),
+               "getAddrAndTypeOfCXXStructor: MS ABI complete destructor");
+  }
+
+  if (!fnType) {
+    if (!fnInfo)
+      fnInfo = &getTypes().arrangeCXXStructorDeclaration(gd);
+    fnType = getTypes().getFunctionType(*fnInfo);
+  }
+
+  auto fn = getOrCreateCIRFunction(getMangledName(gd), fnType, gd,
+                                   /*ForVtable=*/false, dontDefer,
+                                   /*IsThunk=*/false, isForDefinition);
+
+  return {fnType, fn};
+}
+
 cir::FuncOp CIRGenModule::getAddrOfFunction(clang::GlobalDecl gd,
                                             mlir::Type funcType, bool forVTable,
                                             bool dontDefer,
@@ -1248,8 +1295,11 @@ StringRef CIRGenModule::getMangledName(GlobalDecl gd) {
   // Some ABIs don't have constructor variants. Make sure that base and complete
   // constructors get mangled the same.
   if (const auto *cd = dyn_cast<CXXConstructorDecl>(canonicalGd.getDecl())) {
-    errorNYI(cd->getSourceRange(), "getMangledName: C++ constructor");
-    return cast<NamedDecl>(gd.getDecl())->getIdentifier()->getName();
+    if (!getTarget().getCXXABI().hasConstructorVariants()) {
+      errorNYI(cd->getSourceRange(),
+               "getMangledName: C++ constructor without variants");
+      return cast<NamedDecl>(gd.getDecl())->getIdentifier()->getName();
+    }
   }
 
   // Keep the first result in the case of a mangling collision.
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 24ec9ca6403bc..9748c0b3ed43a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -166,11 +166,30 @@ class CIRGenModule : public CIRGenTypeCache {
   mlir::Location getLoc(clang::SourceLocation cLoc);
   mlir::Location getLoc(clang::SourceRange cRange);
 
+  /// Return the best known alignment for an unknown pointer to a
+  /// particular class.
+  clang::CharUnits getClassPointerAlignment(const clang::CXXRecordDecl *rd);
+
   /// FIXME: this could likely be a common helper and not necessarily related
   /// with codegen.
   clang::CharUnits getNaturalTypeAlignment(clang::QualType t,
                                            LValueBaseInfo *baseInfo);
 
+  cir::FuncOp
+  getAddrOfCXXStructor(clang::GlobalDecl gd,
+                       const CIRGenFunctionInfo *fnInfo = nullptr,
+                       cir::FuncType fnType = nullptr, bool dontDefer = false,
+                       ForDefinition_t isForDefinition = NotForDefinition) {
+    return getAddrAndTypeOfCXXStructor(gd, fnInfo, fnType, dontDefer,
+                                       isForDefinition)
+        .second;
+  }
+
+  std::pair<cir::FuncType, cir::FuncOp> getAddrAndTypeOfCXXStructor(
+      clang::GlobalDecl gd, const CIRGenFunctionInfo *fnInfo = nullptr,
+      cir::FuncType fnType = nullptr, bool dontDefer = false,
+      ForDefinition_t isForDefinition = NotForDefinition);
+
   /// This contains all the decls which have definitions but which are deferred
   /// for emission and therefore should only be output if th...
[truncated]

[erichkeane]

I see this is checked, but never set to false?

[andykaylor]

It will be set to false by the code that is missing on line 99.

[erichkeane]

Ah, hrmph... Can we set it there ANYWAY, despite doing an NYI?

[andykaylor]

It will be set to the result of a function call in the final implementation. I can add a line to just set it to false there as a place holder if you think that's useful. Same for the other similar case.

[erichkeane]

ah! Oh, I missed that it is still calculated below. Lets skip it for now then, how you have now LGTM.

[erichkeane]

The only idea I have for us to do something like this, would be to have arrangeCIRFunctionInfo to have a optional arg-transform function. So it becomes:

arrangeCIRFunctionInfo(resultType, argTypes, [&](auto *arg) { astContext.getCanonicalParamType(arg.ty);});

Alternatively, have it take some sort of mutating range, which does the work for us?

Or maybe build that into CallArgList, and have arrangeCIRFunctionInfo take a CallArgList and use a arg.getCanonicalParamType() on CallArg's object. WDYT?

[andykaylor]

This is one of those comments that was brought over from classic codegen. It appears six times in both classic codegen and the CIR incubator codegen, always in conjunction with a call to arrangeLLVMFunctionInfo/arrangeCIRFunctionInfo. The problem is that one of the places it occurs (the static version of arrangeLLVMFunctionInfo/arrangeCIRFunctionInfo) takes a list of types from the caller but then appends additional types based on FunctionProtoType::getExtParameterInfos().

I definitely want to leave it alone for now. It may be a solvable problem, but the priority of solving it appears to be very low.

[erichkeane]

Ah... Its kind of frustrating to see these FIXME's last as long as they do, then get propagated into the new lowering like this. I guess I can live with it (as we have for years), but we gotta stop letting these FIXMEs last as long as they do:/

[andykaylor]

Agreed. We've been following the practice of using "TODO(cir)" when we introduce new placeholders like this in the CIR code so we can distinguish them from TODOs that we copied over from classic codegen, but I expect that they'll have the same problem if we don't do something about it.

[erichkeane]

Don't use auto here?

[erichkeane]

Read, but never set?

[andykaylor]

This will be set by the missing code on line 120.

[erichkeane]

Can we set this anyway? Alternatively, just set it to !d->getInheritedConstructor? Its at least easier here, since we don't need a cast to get the info we need.

[erichkeane]

1 'dont use auto', else everything else LGTM.

[AmrDeveloper]

NIT: arrayType variable will be unused

I think it can be

if (getContext().getAsArrayType(e->getType()))

[AmrDeveloper]

NIT: To eliminate “Do not use else after return” warning, should we remove the else branch and put the code directly 🤔 ?

[andykaylor]

That's just a clang-tidy warning, right?

I was going to say that I'd prefer not to restructure the code that way since the missing code represented by the errorNYI doesn't return in the incubator, but I just looked again, and there is no reason that it couldn't return and that would save a level of indenting on the rest of the code, so I'll go ahead with the suggested change.