[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG27dab4d305ac: Reland Try to implement lambdas with inalloca parameters by forwarding without… (authored by akhuang). Repository: rG LLVM Github

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 544101. akhuang added a comment. add TODO Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007 Files: clang/include/clang/CodeGen/CGFunctionInfo.h

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/CodeGen/CGClass.cpp:3094 + // reason the name doesn't look as expected then just tack __impl to the + // front. + StringRef CallOpName =

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 544090. akhuang added a comment. go back to previous __impl naming method Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007 Files:

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added inline comments. Comment at: clang/lib/CodeGen/CGClass.cpp:3095 + StringRef CallOpName = CallOpFn->getName(); + std::string ImplName = ("__impl" + CallOpName).str(); + akhuang wrote: > rnk wrote: > > akhuang wrote: > > > The old code tried to find

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added inline comments. Comment at: clang/lib/CodeGen/CGClass.cpp:3095 + StringRef CallOpName = CallOpFn->getName(); + std::string ImplName = ("__impl" + CallOpName).str(); + rnk wrote: > akhuang wrote: > > The old code tried to find the "" part of the

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 544086. akhuang added a comment. Add some functions to MicrosoftMangle so we can pass in a custom function name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added inline comments. Comment at: clang/lib/CodeGen/CGClass.cpp:3095 + StringRef CallOpName = CallOpFn->getName(); + std::string ImplName = ("__impl" + CallOpName).str(); + akhuang wrote: > The old code tried to find the "" part of the function name and

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 543038. akhuang added a comment. another fix to function naming code (do the same thing as before except when the mangled name is a hash) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added inline comments. Comment at: clang/lib/CodeGen/CGClass.cpp:3095 + StringRef CallOpName = CallOpFn->getName(); + std::string ImplName = ("__impl" + CallOpName).str(); + The old code tried to find the "" part of the function name and replace the

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 542721. akhuang added a comment. Change impl function naming scheme Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007 Files:

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 539795. akhuang added a comment. minor fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007 Files: clang/include/clang/CodeGen/CGFunctionInfo.h

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5104 +if (CallInfo.isDelegateCall()) { + NeedCopy = false; +} else if (Addr.getAlignment() < Align && rnk wrote: > Please add a comment about this. We need to avoid

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5104 +if (CallInfo.isDelegateCall()) { + NeedCopy = false; +} else if (Addr.getAlignment() < Align && Please add a comment about this. We need to avoid copying

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang updated this revision to Diff 539784. akhuang added a comment. Fix alignment problem Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154007/new/ https://reviews.llvm.org/D154007 Files: clang/include/clang/CodeGen/CGFunctionInfo.h

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1319 + // Check if the invoker is being emitted (could be in any calling conv). + for (CallingConv CC : {CC_C, CC_X86StdCall, CC_X86FastCall, CC_X86ThisCall, CC_X86VectorCall}) +if

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5103 -if (Addr.getAlignment() < Align && +if (CallInfo.isDelegateCall()) { + NeedCopy = false; rnk wrote: > akhuang wrote: > > I think the problem is that it tries

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5103 -if (Addr.getAlignment() < Align && +if (CallInfo.isDelegateCall()) { + NeedCopy = false; akhuang wrote: > I think the problem is that it tries to do a copy here

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang added a comment. In D154007#4457561 , @efriedma wrote: > I'm not confident that isUsed() works the way you want it to in this context. > In particular, if the code in question runs before we've translated the > whole translation unit, the

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added a comment. Can you please add a test case for the issue that caused the revert? I wanted to dig into that to try to understand why the extra copy was being emitted. I think you mentioned it has something to do with increasing the alignment. Repository: rG LLVM Github Monorepo

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

efriedma added a comment. I'm not confident that isUsed() works the way you want it to in this context. In particular, if the code in question runs before we've translated the whole translation unit, the isUsed() bit could change. If you want that's more obviously safe, you could just check

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

akhuang created this revision. Herald added a project: All. akhuang requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reverts commit 8ed7aa59f489715d39d32e72a787b8e75cfda151