rnk added a subscriber: efriedma.
rnk added a comment.
+@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
