Implement lambdas with inalloca parameters by forwarding to function without inalloca calling convention.

[akhuang]

Summary

Lambda functions with inalloca parameters (which are used in 32-bit Windows) currently aren't supported because the call operator delegates to another function and forwards its arguments, and inalloca parameters can't be forwarded. (The same issue exists for variadic arguments.)

This patch emits the lambda call operator again with a different calling convention without inallocas. Then the original call operator and static invoker delegate to this new function. We also modify arrangeLLVMFunctionInfo to be able to generate a different CGFunctionInfo for delegate calls that doesn't use inalloca.

Fixes https://github.com/llvm/llvm-project/issues/28673.

[efriedma]

I'm not quite sure I understand what's happening here. Does this actually avoid generating two copies of the function body if both the call operator and the conversion are used?

[akhuang]

Clean up existing code and add code to make the call operator also call the new function.

[akhuang]

cleanup

[akhuang]

In D137872#3925723, @efriedma wrote:

I'm not quite sure I understand what's happening here. Does this actually avoid generating two copies of the function body if both the call operator and the conversion are used?

That was intended but missing; patch is now updated to make both the invoker and the call operator call the new function.

[rnk]

clang/include/clang/CodeGen/CGFunctionInfo.h

571 ↗

(On Diff #476634)

This is an amusing name, but we should try to find something better. :)

Can we build on this flag to support re-writing a varargs prototype to receive a va_list? If so, we should plan for that, and name this to cover both use cases. Elsewhere in clang we use the terminology "delegate call" see EmitDelegateCallArg. Could this be something like ForDelegation or ForDelegateCall or IsDelegateCall or IsDelegateTarget or something like that? Or Thunk? I think I like ForDelegateCall, but I'm not sure.

---

Does anyone know what a "chain call" is? It wasn't clear to me immediately.

clang/lib/CodeGen/CGCall.cpp

194

This still requires boolean parameters in order. I think LLVM tends to use flag enums for this, something like:

enum class FnInfoOptions {
  None = 0,
  IsInstanceMethod = 1 << 0,
  IsChainCall = 1 << 1,
  IsDelegateCall = 1 << 2,
};

Call sites look like:

arrangeLLVMFunctionInfo(RetTy, FnInfoOptions::None, None, FTNP->getExtInfo()...)
arrangeLLVMFunctionInfo(RetTy, FnInfoOptions::IsInstanceMethod, None, FTNP->getExtInfo()...)

clang/lib/CodeGen/CGClass.cpp

3012

I believe this is only necessary for x86 platforms, so we should check the architecture too to avoid the extra work on x64.

3156

We should use a name which demangles correctly, as we do for __invoke. This is a good first draft, though.

clang/test/CodeGenCXX/inalloca-lambda.cpp

4 ↗

(On Diff #476634)

Just to make the IR slightly nicer, give this an int member.

8 ↗

(On Diff #476634)

Please expand the testing further to include a case where the same lambda is called directly and goes through function pointer decay, something like this:
https://gcc.godbolt.org/z/q7x3ov5e1

This is to ensure that we don't end up double-emitting the implementation of the lambda.

[akhuang]

add to test case, modify name mangling, change fn info opts enum type

[rnk]

clang/lib/CodeGen/CGCall.cpp

735

We could avoid static_cast by defining the standard flag operators, similar to what we do in this macro:
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/DebugInfo/CodeView/CodeView.h#L52

Alternatively, just define operator| and operator&.

clang/lib/CodeGen/TargetInfo.cpp

1190 ↗

(On Diff #477289)

Please update the old parameter name

1841 ↗

(On Diff #477289)

As a simplification, you can put || isDelegateCall here, we're basically pretending that this returns RAA_Indirect.

[akhuang]

Address comments

[akhuang]

ping @efriedma, do you mind looking at this again? thanks!

clang/include/clang/CodeGen/CGFunctionInfo.h

571 ↗

(On Diff #476634)

Yeah, that naming makes sense.

chain call: https://reviews.llvm.org/D6332?

clang/lib/CodeGen/CGClass.cpp

3156

Fixed to replace different part of the mangled name with __impl. Don't know if it would be better to use the actual mangling functions, but the string replace here seems simple enough.

[akhuang]

whoops, I've left this here for a while.. @efriedma, are you able to review it?

[efriedma]

I'm having a bit of trouble following how exactly the thunk creation is working here... do we generate different code depending on whether the call operator and/or the static invoker are referenced? Why is the function in EmitLambdaInAllocaCallOpFn not getting defined using the normal CodeGenModule machinery?

clang/lib/CodeGen/CGCall.cpp

770

I think you need to pass isDelegateCall to CGFunctionInfo::Profile.

[akhuang]

In D137872#4327615, @efriedma wrote:

I'm having a bit of trouble following how exactly the thunk creation is working here... do we generate different code depending on whether the call operator and/or the static invoker are referenced?

So I think it used to be that the static invoker calls the call operator which contains the body of the lambda? And now both the static invoker and the call operator are delegating to this new __impl function. In EmitLambdaStaticInvokeBody it first calls EmitLambdaInAllocaCallOpFn to make the new call operator (which is populated using EmitLambdaDelegatingInvokeBody). And then inside EmitLambdaDelegatingInvokeBody it generates the new impl function and calls it in the body. Not sure if that answers the question, I agree the code is a bit roundabout.

Why is the function in EmitLambdaInAllocaCallOpFn not getting defined using the normal CodeGenModule machinery?

Do you mean why it's not using CodeGenModule machinery to generate the new call op body or why we're changing the original call op body?

[efriedma]

In D137872#4348314, @akhuang wrote:

In D137872#4327615, @efriedma wrote:

I'm having a bit of trouble following how exactly the thunk creation is working here... do we generate different code depending on whether the call operator and/or the static invoker are referenced?

So I think it used to be that the static invoker calls the call operator which contains the body of the lambda? And now both the static invoker and the call operator are delegating to this new __impl function. In EmitLambdaStaticInvokeBody it first calls EmitLambdaInAllocaCallOpFn to make the new call operator (which is populated using EmitLambdaDelegatingInvokeBody). And then inside EmitLambdaDelegatingInvokeBody it generates the new impl function and calls it in the body. Not sure if that answers the question, I agree the code is a bit roundabout.

Why is the function in EmitLambdaInAllocaCallOpFn not getting defined using the normal CodeGenModule machinery?

Do you mean why it's not using CodeGenModule machinery to generate the new call op body or why we're changing the original call op body?

I mean why it's not using the CodeGenModule machinery. I understand there are three function bodies involved. But I would expect that when you "emit" the lambda call operator, it emits the entry point for the call and for the call impl, and when you emit the static invoker, it just emits a call to the call impl.

Or... hmm. Is this actually erasing the existing definition of the call operator, if it was already emitted? That probably doesn't actually work in general; once a function is emitted, you can't re-emit it. Emitting a function emits other related stuff like static variables; you can't EmitFunctionBody() more than once. Either you need to decide when it's first emitted, or you need to rewrite the signature of the definition and splice the existing body in.

[akhuang]

• move call operator emission into the path in GenerateCode
• formatting

[akhuang]

remove print statements

[akhuang]

In D137872#4357268, @efriedma wrote:

In D137872#4348314, @akhuang wrote:

In D137872#4327615, @efriedma wrote:

I'm having a bit of trouble following how exactly the thunk creation is working here... do we generate different code depending on whether the call operator and/or the static invoker are referenced?

So I think it used to be that the static invoker calls the call operator which contains the body of the lambda? And now both the static invoker and the call operator are delegating to this new __impl function. In EmitLambdaStaticInvokeBody it first calls EmitLambdaInAllocaCallOpFn to make the new call operator (which is populated using EmitLambdaDelegatingInvokeBody). And then inside EmitLambdaDelegatingInvokeBody it generates the new impl function and calls it in the body. Not sure if that answers the question, I agree the code is a bit roundabout.

Why is the function in EmitLambdaInAllocaCallOpFn not getting defined using the normal CodeGenModule machinery?

Do you mean why it's not using CodeGenModule machinery to generate the new call op body or why we're changing the original call op body?

I mean why it's not using the CodeGenModule machinery. I understand there are three function bodies involved. But I would expect that when you "emit" the lambda call operator, it emits the entry point for the call and for the call impl, and when you emit the static invoker, it just emits a call to the call impl.

Or... hmm. Is this actually erasing the existing definition of the call operator, if it was already emitted? That probably doesn't actually work in general; once a function is emitted, you can't re-emit it. Emitting a function emits other related stuff like static variables; you can't EmitFunctionBody() more than once. Either you need to decide when it's first emitted, or you need to rewrite the signature of the definition and splice the existing body in.

good point, I realized I can just put the call operator emission where the call op is normally emitted, which removes the thing where I was trying to create a new call op while the static invoker body is being emitted.

[akhuang]

more cleanup

[akhuang]

Fix ordering in the test case

[efriedma]

clang/lib/CodeGen/CGClass.cpp

3174

Is there any way we can use GenerateCode as the entrypoint here, instead of calling EmitFunctionBody directly? Without this patch, EmitFunctionBody has exactly one caller, so this makes the control flow and special cases more complicated to reason about. For example, there's some special coroutine handling in GenerateCode.

clang/lib/CodeGen/CodeGenFunction.cpp

1472 ↗

(On Diff #529780)

Does this pass the correct value of "this"? EmitLambdaStaticInvokeBody creates a new alloca to represent "this", but it's already an argument to the function.

Granted, it only matters in really obscure cases, but still.

[akhuang]

clang/lib/CodeGen/CGClass.cpp

3174

Changed to using GenerateCode -- had to add another case to the path in GenerateCode to make sure we emit different things for the call op body inside __impl and the call op body inside the call op function.

clang/lib/CodeGen/CodeGenFunction.cpp

1472 ↗

(On Diff #529780)

That's true, the "this" won't be passed correctly.

Actually, would it be fine to just emit the original call op body? (so that the same function body is emitted twice -- once in the call op and once in __impl).

[akhuang]

Call GenerateCode to emit __impl function body

[efriedma]

clang/lib/CodeGen/CodeGenFunction.cpp

1472 ↗

(On Diff #529780)

Not completely sure what you're asking... but as I've mentioned, we can't EmitFunctionBody() the body of a function more than once.

[akhuang]

Emit call op which forwards %this argument

clang/lib/CodeGen/CodeGenFunction.cpp

1472 ↗

(On Diff #529780)

Oh, nevermind then. I think this version passes "this" from the call op.

[akhuang]

update test case

[efriedma]

clang/lib/CodeGen/CodeGenFunction.cpp

1470 ↗

(On Diff #531571)

I'm not sure __impl is unique enough that user code will never use it. (I mean, it's reserved, but we don't actually forbid using it.)

Can we use the isDelegateCall() bit?

[akhuang]

Use isDelegateCall to check whether function is actual call op fn or not

clang/lib/CodeGen/CodeGenFunction.cpp

1470 ↗

(On Diff #531571)

yep, that makes more sense.

[efriedma]

LGTM

[rnk]

I'm really happy to see this limitation finally addressed! It's quite a bit of embarrassing tech debt that affects folks making Windows x86 builds.

[akhuang]

rebase and clang format

[hans]

For those following along, this was relanded in https://reviews.llvm.org/D154007

clang/lib/CodeGen/CGCall.cpp

arrangeLLVMFunctionInfo(CodeGenTypes &CGT, bool instanceMethod,
   RequiredArgs Required = RequiredArgs::forPrototypePlus(FTP, prefix.size());
   // FIXME: Kill copy.
   appendParameterTypes(CGT, prefix, paramInfos, FTP);
   CanQualType resultType = FTP->getReturnType().getUnqualifiedType();
 
   return CGT.arrangeLLVMFunctionInfo(resultType, instanceMethod,
                                      /*chainCall=*/false, prefix,
                                      FTP->getExtInfo(), paramInfos,
                                      Required);
 }
 
 /// Arrange the argument and result information for a value of the
 /// given freestanding function type.
 const CGFunctionInfo &
 CodeGenTypes::arrangeFreeFunctionType(CanQual<FunctionProtoType> FTP) {
   SmallVector<CanQualType, 16> argTypes;
   return ::arrangeLLVMFunctionInfo(*this, /*instanceMethod=*/false, argTypes,
if (!sigParamInfos.empty()) {
     paramInfos.resize(args.size());
   }
 
   auto argTypes = getArgTypesForCall(Context, args);
 
   assert(signature.getRequiredArgs().allowsOptionalArgs());
   return arrangeLLVMFunctionInfo(signature.getReturnType(),
                                  signature.isInstanceMethod(),
                                  signature.isChainCall(),
                                  argTypes,
                                  signature.getExtInfo(),
                                  paramInfos,
                                  signature.getRequiredArgs());
 }
 
 namespace clang {
 namespace CodeGen {
CodeGenTypes::arrangeLLVMFunctionInfo(CanQualType resultType,
   // Lookup or create unique function info.
   llvm::FoldingSetNodeID ID;
   CGFunctionInfo::Profile(ID, instanceMethod, chainCall, info, paramInfos,
                           required, resultType, argTypes);
 
   void *insertPos = nullptr;
   CGFunctionInfo *FI = FunctionInfos.FindNodeOrInsertPos(ID, insertPos);
   if (FI)
     return *FI;
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
                               paramInfos, resultType, argTypes, required);
   FunctionInfos.InsertNode(FI, insertPos);
 
void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
                                           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 (isInAllocaArgument(CGM.getCXXABI(), type)) {
-    CGM.ErrorUnsupported(param, "forwarded non-trivially copyable parameter");
-  }
-
   // 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(local)), type);
 
   // In ARC, move out of consumed arguments so that the release cleanup
   // entered by StartFunction doesn't cause an over-release.  This isn't
   // optimal -O0 code generation, but it should get cleaned up when

clang/lib/CodeGen/CGClass.cpp

void CodeGenFunction::EmitLambdaBlockInvokeBody() {
   for (auto *param : BD->parameters())
     EmitDelegateCallArg(CallArgs, param, param->getBeginLoc());
 
   assert(!Lambda->isGenericLambda() &&
             "generic lambda interconversion to block not implemented");
   EmitForwardingCallToLambda(CallOp, CallArgs);
 }
 
+static bool isInAllocaArg(CGCXXABI &ABI, QualType T) {
+  const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
+  return RD && ABI.getRecordArgABI(RD) == CGCXXABI::RAA_DirectInMemory;
+}
+
+static bool hasInAllocaArg(const TargetInfo &TI, CGCXXABI &ABI, const CXXMethodDecl *MD) {
+  return TI.getCXXABI().isMicrosoft() &&
+    llvm::any_of(MD->parameters(), [&](ParmVarDecl *P) {
+          return isInAllocaArg(ABI, P->getType());
+        });
+}
+
 void CodeGenFunction::EmitLambdaDelegatingInvokeBody(const CXXMethodDecl *MD) {
   const CXXRecordDecl *Lambda = MD->getParent();
 
   // Start building arguments for forwarding call
   CallArgList CallArgs;
+  SmallVector<llvm::Value *> ArgList;
 
   QualType LambdaType = getContext().getRecordType(Lambda);
   QualType ThisType = getContext().getPointerType(LambdaType);
   Address ThisPtr = CreateMemTemp(LambdaType, "unused.capture");
   CallArgs.add(RValue::get(ThisPtr.getPointer()), ThisType);
+  ArgList.push_back(ThisPtr.getPointer());
 
   // Add the rest of the parameters.
-  for (auto *Param : MD->parameters())
+  for (auto *Param : MD->parameters()) {
     EmitDelegateCallArg(CallArgs, Param, Param->getBeginLoc());
 
+    if (Param->getType()->isReferenceType())
+      ArgList.push_back(Builder.CreateLoad(GetAddrOfLocalVar(Param)));
+    else
+      ArgList.push_back(GetAddrOfLocalVar(Param).getPointer());
+  }
+
   const CXXMethodDecl *CallOp = Lambda->getLambdaCallOperator();
   // For a generic lambda, find the corresponding call operator specialization
   // to which the call to the static-invoker shall be forwarded.
   if (Lambda->isGenericLambda()) {
     assert(MD->isFunctionTemplateSpecialization());
     const TemplateArgumentList *TAL = MD->getTemplateSpecializationArgs();
     FunctionTemplateDecl *CallOpTemplate = CallOp->getDescribedFunctionTemplate();
     void *InsertPos = nullptr;
     FunctionDecl *CorrespondingCallOpSpecialization =
         CallOpTemplate->findSpecialization(TAL->asArray(), InsertPos);
     assert(CorrespondingCallOpSpecialization);
     CallOp = cast<CXXMethodDecl>(CorrespondingCallOpSpecialization);
   }
+
+  // Special lambda forwarding when there are inalloca parameters.
+  if (hasInAllocaArg(getTarget(), CGM.getCXXABI(), MD)) {
+    EmitInAllocaForwardingCallToLambda(MD, CallOp, ArgList, CallArgs);
+    return;
+  }
+
   EmitForwardingCallToLambda(CallOp, CallArgs);
 }
 
+void CodeGenFunction::EmitInAllocaForwardingCallToLambda(
+    const CXXMethodDecl *MD, const CXXMethodDecl *CallOp,
+    ArrayRef<llvm::Value *> ArgList,
+    CallArgList &CallArgs) {
+  const CGFunctionInfo &FnInfo =
+    CGM.getTypes().arrangeCXXMethodDeclaration(CallOp);
+
+  // Create new call op function with different calling convention.
+  CodeGenFunction CGF(CGM);
+  CGF.CurGD = GlobalDecl(CallOp);
+  llvm::Function *CallOpFn =
+    cast<llvm::Function>(CGM.GetAddrOfFunction(GlobalDecl(CallOp)));
+
+  std::string ImplName = (CallOpFn->getName() + ".impl").str();
+  llvm::Function *ImplFn = CallOpFn->getParent()->getFunction(ImplName);
+  if (!ImplFn) {
+    // Generate the function.
+    ImplFn = llvm::Function::Create(CGM.getTypes().GetFunctionType(FnInfo), 
+                                    llvm::GlobalValue::InternalLinkage,
+                                    ImplName, CGM.getModule());
+    ImplFn->setCallingConv(llvm::CallingConv::C);
+    CGF.GenerateCode(CGF.CurGD, ImplFn, FnInfo);
+    CGM.SetLLVMFunctionAttributesForDefinition(CallOp, ImplFn);
+  }
+
+  // Deactivate any cleanups that we're supposed to do immediately before
+  // the call.
+  if (!CallArgs.getCleanupsToDeactivate().empty()) {
+    ArrayRef<CallArgList::CallArgCleanup> Cleanups =
+      CallArgs.getCleanupsToDeactivate();
+    for (const auto &I : llvm::reverse(Cleanups)) {
+      DeactivateCleanupBlock(I.Cleanup, I.IsActiveIP);
+      I.IsActiveIP->eraseFromParent();
+    }
+  }
+
+  // Get the args to pass to the call op. This only works because we're assuming
+  // there are two arguments; this, and the inalloca one.
+  SmallVector<llvm::Value *, 2> NewArgList;
+  NewArgList.push_back(ArgList[0]); // Add the %this arg.
+  CGFunctionInfo::const_arg_iterator info_it = FnInfo.arg_begin();
+  for (auto I = ArgList.begin(); I != ArgList.end(); ++I, ++info_it) {
+    const ABIArgInfo &ArgInfo = info_it->info;
+    if (ArgInfo.getKind() == ABIArgInfo::InAlloca) {
+      NewArgList.push_back(*I); // Add the inalloca arg.
+      break;
+    }
+  }
+
+  // Create the call.
+  llvm::CallInst *Call = Builder.CreateCall(ImplFn, ArgList);
+  Call->setCallingConv(ImplFn->getCallingConv());
+
+  QualType RetTy = FnInfo.getReturnType();
+  ReturnValueSlot ReturnSlot;
+
+  // If necessary, copy the returned value into the slot.
+  if (!RetTy->isVoidType() && ReturnSlot.isNull())
+    EmitReturnOfRValue(RValue::get(Call), RetTy);
+  else
+    EmitBranchThroughCleanup(ReturnBlock);
+
+  return;
+}
+
 void CodeGenFunction::EmitLambdaStaticInvokeBody(const CXXMethodDecl *MD) {
   if (MD->isVariadic()) {
     // FIXME: Making this work correctly is nasty because it requires either
     // cloning the body of the call operator or making the call operator forward.
     CGM.ErrorUnsupported(MD, "lambda conversion to variadic function");
     return;
   }
-
   EmitLambdaDelegatingInvokeBody(MD);
 }

clang/lib/CodeGen/CodeGenFunction.h

public:
   void EmitFunctionBody(const Stmt *Body);
   void EmitBlockWithFallThrough(llvm::BasicBlock *BB, const Stmt *S);
 
   void EmitForwardingCallToLambda(const CXXMethodDecl *LambdaCallOperator,
                                   CallArgList &CallArgs);
   void EmitLambdaBlockInvokeBody();
   void EmitLambdaDelegatingInvokeBody(const CXXMethodDecl *MD);
   void EmitLambdaStaticInvokeBody(const CXXMethodDecl *MD);
+  void EmitInAllocaForwardingCallToLambda(const CXXMethodDecl *MD, 
+                                          const CXXMethodDecl *CallOp, 
+                                          ArrayRef<llvm::Value *> ArgList,
+                                          CallArgList &CallArgs);
   void EmitLambdaVLACapture(const VariableArrayType *VAT, LValue LV) {
     EmitStoreThroughLValue(RValue::get(VLASizeMap[VAT->getSizeExpr()]), LV);
   }
   void EmitAsanPrologueOrEpilogue(bool Prologue);
 
   /// Emit the unified return block, trying to avoid its emission when
   /// possible.
   /// \return The debug location of the user written return statement if the