[CIR][NFC] Use arrangeFunctionDeclaration to build function types

[andykaylor]

This change replaces the simplified call that we were previously using to convert the function type provided by a global declaration to the CIR function type. We now go through 'arrangeGlobalDeclaration' which builds the function type in a more complicated manner. This change has no observable differences for the currently upstreamed CIR support, but it is necessary to prepare for C++ member function calls, which require the extra handling.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This change replaces the simplified call that we were previously using to convert the function type provided by a global declaration to the CIR function type. We now go through 'arrangeGlobalDeclaration' which builds the function type in a more complicated manner. This change has no observable differences for the currently upstreamed CIR support, but it is necessary to prepare for C++ member function calls, which require the extra handling.

---

Full diff: https://github.com/llvm/llvm-project/pull/139787.diff

5 Files Affected:

• (modified) clang/lib/CIR/CodeGen/CIRGenCall.cpp (+62-14)
• (modified) clang/lib/CIR/CodeGen/CIRGenFunctionInfo.h (+13-3)
• (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+2-15)
• (modified) clang/lib/CIR/CodeGen/CIRGenTypes.cpp (+14)
• (modified) clang/lib/CIR/CodeGen/CIRGenTypes.h (+30-2)

diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
index 5c65a43641844..9ad048bf357cc 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
@@ -44,14 +44,11 @@ CIRGenFunctionInfo::create(CanQualType resultType,
 
 cir::FuncType CIRGenTypes::getFunctionType(const CIRGenFunctionInfo &fi) {
   mlir::Type resultType = convertType(fi.getReturnType());
+  SmallVector<mlir::Type, 8> argTypes;
+  argTypes.reserve(fi.getNumRequiredArgs());
 
-  SmallVector<mlir::Type, 8> argTypes(fi.getNumRequiredArgs());
-
-  unsigned argNo = 0;
-  llvm::ArrayRef<CIRGenFunctionInfoArgInfo> argInfos(fi.argInfoBegin(),
-                                                     fi.getNumRequiredArgs());
-  for (const auto &argInfo : argInfos)
-    argTypes[argNo++] = convertType(argInfo.type);
+  for (const CIRGenFunctionInfoArgInfo &argInfo : fi.requiredArguments())
+    argTypes.push_back(convertType(argInfo.type));
 
   return cir::FuncType::get(argTypes,
                             (resultType ? resultType : builder.getVoidTy()),
@@ -63,6 +60,34 @@ CIRGenCallee CIRGenCallee::prepareConcreteCallee(CIRGenFunction &cgf) const {
   return *this;
 }
 
+/// 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
+static void appendParameterTypes(const CIRGenTypes &cgt,
+                                 SmallVectorImpl<CanQualType> &prefix,
+                                 CanQual<FunctionProtoType> fpt) {
+  assert(!cir::MissingFeatures::opCallExtParameterInfo());
+  // Fast path: don't touch param info if we don't need to.
+  if (!fpt->hasExtParameterInfos()) {
+    prefix.append(fpt->param_type_begin(), fpt->param_type_end());
+    return;
+  }
+
+  cgt.getCGModule().errorNYI("appendParameterTypes: hasExtParameterInfos");
+}
+
+/// Arrange the CIR function layout for a value of the given function type, on
+/// top of any implicit parameters already stored.
+static const CIRGenFunctionInfo &
+arrangeCIRFunctionInfo(CIRGenTypes &cgt, SmallVectorImpl<CanQualType> &prefix,
+                       CanQual<FunctionProtoType> ftp) {
+  RequiredArgs required = RequiredArgs::forPrototypePlus(ftp, prefix.size());
+  assert(!cir::MissingFeatures::opCallExtParameterInfo());
+  appendParameterTypes(cgt, prefix, ftp);
+  CanQualType resultType = ftp->getReturnType().getUnqualifiedType();
+  return cgt.arrangeCIRFunctionInfo(resultType, prefix, required);
+}
+
 static const CIRGenFunctionInfo &
 arrangeFreeFunctionLikeCall(CIRGenTypes &cgt, CIRGenModule &cgm,
                             const CallArgList &args,
@@ -95,6 +120,34 @@ CIRGenTypes::arrangeFreeFunctionCall(const CallArgList &args,
   return arrangeFreeFunctionLikeCall(*this, cgm, args, fnType);
 }
 
+/// Arrange the argument and result information for the declaration or
+/// definition of the given function.
+const CIRGenFunctionInfo &
+CIRGenTypes::arrangeFunctionDeclaration(const FunctionDecl *fd) {
+  if (const auto *md = dyn_cast<CXXMethodDecl>(fd)) {
+    if (md->isInstance()) {
+      cgm.errorNYI("arrangeFunctionDeclaration: instance method");
+    }
+  }
+
+  CanQualType funcTy = fd->getType()->getCanonicalTypeUnqualified();
+
+  assert(isa<FunctionType>(funcTy));
+  // TODO: setCUDAKernelCallingConvention
+  assert(!cir::MissingFeatures::cudaSupport());
+
+  // When declaring a function without a prototype, always use a non-variadic
+  // type.
+  if (CanQual<FunctionNoProtoType> noProto =
+          funcTy.getAs<FunctionNoProtoType>()) {
+    assert(!cir::MissingFeatures::opCallCIRGenFuncInfoExtParamInfo());
+    return arrangeCIRFunctionInfo(noProto->getReturnType(), std::nullopt,
+                                  RequiredArgs::All);
+  }
+
+  return arrangeFreeFunctionType(funcTy.castAs<FunctionProtoType>());
+}
+
 static cir::CIRCallOpInterface
 emitCallLikeOp(CIRGenFunction &cgf, mlir::Location callLoc,
                cir::FuncOp directFuncOp,
@@ -112,13 +165,8 @@ emitCallLikeOp(CIRGenFunction &cgf, mlir::Location callLoc,
 
 const CIRGenFunctionInfo &
 CIRGenTypes::arrangeFreeFunctionType(CanQual<FunctionProtoType> fpt) {
-  SmallVector<CanQualType, 8> argTypes;
-  for (unsigned i = 0, e = fpt->getNumParams(); i != e; ++i)
-    argTypes.push_back(fpt->getParamType(i));
-  RequiredArgs required = RequiredArgs::forPrototypePlus(fpt);
-
-  CanQualType resultType = fpt->getReturnType().getUnqualifiedType();
-  return arrangeCIRFunctionInfo(resultType, argTypes, required);
+  SmallVector<CanQualType, 16> argTypes;
+  return ::arrangeCIRFunctionInfo(*this, argTypes, fpt);
 }
 
 const CIRGenFunctionInfo &
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunctionInfo.h b/clang/lib/CIR/CodeGen/CIRGenFunctionInfo.h
index 0556408fb98d1..3b3ac0a6f5194 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunctionInfo.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunctionInfo.h
@@ -47,7 +47,8 @@ class RequiredArgs {
   ///
   /// If FD is not null, this will consider pass_object_size params in FD.
   static RequiredArgs
-  forPrototypePlus(const clang::FunctionProtoType *prototype) {
+  forPrototypePlus(const clang::FunctionProtoType *prototype,
+                   unsigned additional) {
     if (!prototype->isVariadic())
       return All;
 
@@ -58,8 +59,9 @@ class RequiredArgs {
   }
 
   static RequiredArgs
-  forPrototypePlus(clang::CanQual<clang::FunctionProtoType> prototype) {
-    return forPrototypePlus(prototype.getTypePtr());
+  forPrototypePlus(clang::CanQual<clang::FunctionProtoType> prototype,
+                   unsigned additional) {
+    return forPrototypePlus(prototype.getTypePtr(), additional);
   }
 
   unsigned getNumRequiredArgs() const {
@@ -114,6 +116,14 @@ class CIRGenFunctionInfo final
     getReturnType().Profile(id);
   }
 
+  llvm::ArrayRef<ArgInfo> arguments() const {
+    return llvm::ArrayRef<ArgInfo>(argInfoBegin(), numArgs);
+  }
+
+  llvm::ArrayRef<ArgInfo> requiredArguments() const {
+    return llvm::ArrayRef<ArgInfo>(argInfoBegin(), getNumRequiredArgs());
+  }
+
   CanQualType getReturnType() const { return getArgsBuffer()[0].type; }
 
   const_arg_iterator argInfoBegin() const { return getArgsBuffer() + 1; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index b4e27bc5fec6a..bd3aa37a92af6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -249,21 +249,8 @@ void CIRGenModule::emitGlobalFunctionDefinition(clang::GlobalDecl gd,
     return;
   }
 
-  cir::FuncType funcType;
-  // TODO: Move this to arrangeFunctionDeclaration when it is
-  // implemented.
-  // When declaring a function without a prototype, always use a
-  // non-variadic type.
-  if (CanQual<FunctionNoProtoType> noProto =
-          funcDecl->getType()
-              ->getCanonicalTypeUnqualified()
-              .getAs<FunctionNoProtoType>()) {
-    const CIRGenFunctionInfo &fi = getTypes().arrangeCIRFunctionInfo(
-        noProto->getReturnType(), {}, RequiredArgs::All);
-    funcType = getTypes().getFunctionType(fi);
-  } else {
-    funcType = cast<cir::FuncType>(convertType(funcDecl->getType()));
-  }
+  const CIRGenFunctionInfo &fi = getTypes().arrangeGlobalDeclaration(gd);
+  cir::FuncType funcType = getTypes().getFunctionType(fi);
 
   cir::FuncOp funcOp = dyn_cast_if_present<cir::FuncOp>(op);
   if (!funcOp || funcOp.getFunctionType() != funcType) {
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index 097d14b370940..dc8872122995c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -553,3 +553,17 @@ CIRGenTypes::arrangeCIRFunctionInfo(CanQualType returnType,
 
   return *fi;
 }
+
+const CIRGenFunctionInfo &CIRGenTypes::arrangeGlobalDeclaration(GlobalDecl gd) {
+  assert(!dyn_cast<ObjCMethodDecl>(gd.getDecl()) &&
+         "This is reported as a FIXME in LLVM codegen");
+  const auto *fd = cast<FunctionDecl>(gd.getDecl());
+
+  if (isa<CXXConstructorDecl>(gd.getDecl()) ||
+      isa<CXXDestructorDecl>(gd.getDecl())) {
+    cgm.errorNYI(SourceLocation(),
+                 "arrangeGlobalDeclaration for C++ constructor or destructor");
+  }
+
+  return arrangeFunctionDeclaration(fd);
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.h b/clang/lib/CIR/CodeGen/CIRGenTypes.h
index ff8ce3f87f362..53e79c3641c40 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.h
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.h
@@ -117,6 +117,36 @@ class CIRGenTypes {
   // TODO: convert this comment to account for MLIR's equivalence
   mlir::Type convertTypeForMem(clang::QualType, bool forBitField = false);
 
+  /// Get the CIR function type for \arg Info.
+  cir::FuncType getFunctionType(const CIRGenFunctionInfo &info);
+
+  // The arrangement methods are split into three families:
+  //   - those meant to drive the signature and prologue/epilogue
+  //     of a function declaration or definition,
+  //   - those meant for the computation of the CIR type for an abstract
+  //     appearance of a function, and
+  //   - those meant for performing the CIR-generation of a call.
+  // They differ mainly in how they deal with optional (i.e. variadic)
+  // arguments, as well as unprototyped functions.
+  //
+  // Key points:
+  // - The CIRGenFunctionInfo for emitting a specific call site must include
+  //   entries for the optional arguments.
+  // - The function type used at the call site must reflect the formal
+  // signature
+  //   of the declaration being called, or else the call will go away.
+  // - For the most part, unprototyped functions are called by casting to a
+  //   formal signature inferred from the specific argument types used at the
+  //   call-site. However, some targets (e.g. x86-64) screw with this for
+  //   compatability reasons.
+
+  const CIRGenFunctionInfo &arrangeGlobalDeclaration(GlobalDecl gd);
+
+  /// Free functions are functions that are compatible with an ordinary C
+  /// function pointer type.
+  const CIRGenFunctionInfo &
+  arrangeFunctionDeclaration(const clang::FunctionDecl *fd);
+
   /// Return whether a type can be zero-initialized (in the C++ sense) with an
   /// LLVM zeroinitializer.
   bool isZeroInitializable(clang::QualType ty);
@@ -134,8 +164,6 @@ class CIRGenTypes {
   arrangeFreeFunctionType(CanQual<FunctionProtoType> fpt);
   const CIRGenFunctionInfo &
   arrangeFreeFunctionType(CanQual<FunctionNoProtoType> fnpt);
-
-  cir::FuncType getFunctionType(const CIRGenFunctionInfo &fi);
 };
 
 } // namespace clang::CIRGen

[erichkeane]

Isn't that what the cudaSupport assert means?

[andykaylor]

It is. The comment was in the code I was porting, and is a bit more specific, so I decided to keep it, but the assert provides a more functional placeholder, so I added that too.

[erichkeane]

Can you decode this name for me? it doesn't really make sense to me.

[andykaylor]

Yeah, it doesn't make sense yet because the implementation is still incomplete. This function returns a RequiredArgs object representing the number of arguments required by the function prototype plus additional slots for a number of arguments specified by the additional parameter. The previous implementation omitted the additional parameter because we didn't need it. I added it because it was referenced by one of the functions I'm adding here, but it will always be zero with the code I added.

When the support for C++ member functions gets added, this will reserve a slot for the this argument. Eventually, it will be used in a few other places, such as suffix arguments for constructors and chain calls.