[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: efriedma.
rnk added a comment.

+ at efriedma, can you review this as a Clang CodeGen owner, according to the recently updated CodeOwners.rst file?
https://github.com/llvm/llvm-project/blob/main/clang/CodeOwners.rst#clang-llvm-ir-generation



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3590-3593
+          getTarget().getCXXABI().isMicrosoft() &&
+          llvm::any_of(cast<CXXMethodDecl>(FD)->parameters(), [&](ParmVarDecl *P) {
+            return isInAllocaArgument(getCXXABI(), P->getType());
+            })) {
----------------
akhuang wrote:
> akhuang wrote:
> > rnk wrote:
> > > akhuang wrote:
> > > > rnk wrote:
> > > > > For simplicity, what if we always emitted the call operator for all lambda static invokers into the IR first? So, this logic would then become almost exactly the same as the emitCXXStructor logic above.
> > > > > 
> > > > > Later, in EmitLambdaStaticInvokeBody, we can detect the inalloca case and start the cloning.
> > > > > For simplicity, what if we always emitted the call operator for all lambda static invokers into the IR first? So, this logic would then become almost exactly the same as the emitCXXStructor logic above.
> > > > 
> > > > Do you know where this should happen? I couldn't really figure out a place other than here for emitting the call operator. 
> > > > 
> > > > If I do the cloning inside the normal EmitLambdaStaticInvokeBody path it's a bit annoying because StartFunction/EndFunction get called before/after cloning.
> > > > Do you know where this should happen? I couldn't really figure out a place other than here for emitting the call operator.
> > > 
> > > Yes, I think you should do that here, just like we do for constructors. If it's a hack, it's one we already have. The main impact is that we emit the call operator in the IR first. That may require updating some tests, but it's nice to do the same thing on all platforms, and we'd need to do it to handle forwarding varargs lambdas anyway, which are present on all platforms.
> > > 
> > > I also think it's OK to delete all the IR from StartFunction after its been generated, that doesn't seem like a big deal. How does the varargs cloning logic handle this situation?
> > Oh, ok, I see what you mean. 
> > 
> > I'll try to upload a version with the cloning function inside EmitLambdaStaticInvokeBody. I think there's some stuff in Start/End Function that prevent you from simply deleting the code. (I don't think it's an issue for the varargs cloning because that isn't called within `EmitGlobalFunctionDefinition`. )
> Actually, hm, trying to do the function cloning inside `EmitLambdaStaticInvokeBody`/`GenerateCode` might not work because `FinishFunction` does various things like emit the return statement, which won't work if we just cloned the function.
If it doesn't work at all, I'm OK with doing this here, but I would like to try to move as much logic as possible out of `EmitGlobalDefinition`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136998/new/

https://reviews.llvm.org/D136998