akhuang added inline comments.
================
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());
+ })) {
----------------
rnk wrote:
> 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`.
Yeah, it doesn't seem ideal.. I guess the logic can also be moved into
`EmitGlobalFunctionDefinition` (or a new CodeGenModule wrapper function) but
that doesn't seem much better.
I don't think cloning the function in `GenerateCode` between StartFunction and
FinishFunction will work though. Otherwise maybe can go back to trying a
different method like changing the CXXMethodDecl from the call op and emitting
it through the normal codegen path.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136998/new/
https://reviews.llvm.org/D136998
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
