[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

This revision was automatically updated to reflect the committed changes. Closed by commit rGec809e4cfe0b: PR47372: Fix Lambda invoker calling conventions (authored by erichkeane). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Alright, LGTM. Comment at: clang/include/clang/AST/DeclCXX.h:1013 CXXMethodDecl *getLambdaStaticInvoker() const; + CXXMethodDecl *getLambdaStaticInvoker(CallingConv

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane updated this revision to Diff 301746. erichkeane marked an inline comment as done. erichkeane added a comment. @rjmccall : Think I got everything, is this what you mean? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89559/new/ https://reviews.llvm.org/D89559 Files:

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1013 CXXMethodDecl *getLambdaStaticInvoker() const; + CXXMethodDecl *getLambdaStaticInvoker(CallingConv CC) const; + erichkeane wrote: > rjmccall wrote: > > Probably worth

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1013 CXXMethodDecl *getLambdaStaticInvoker() const; + CXXMethodDecl *getLambdaStaticInvoker(CallingConv CC) const; + rjmccall wrote: > Probably worth clarifying in the comment

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1013 CXXMethodDecl *getLambdaStaticInvoker() const; + CXXMethodDecl *getLambdaStaticInvoker(CallingConv CC) const; + Probably worth clarifying in the comment which invoker is

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89559/new/ https://reviews.llvm.org/D89559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane updated this revision to Diff 300396. erichkeane added a comment. Alright, I have the multi-emit dealt with, and the problems that come with that solved. This doesn't do the MSVC-emit-for-all-CCs thing, but I intend to do that in a separate patch with the same infrastructure.

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a subscriber: majnemer. erichkeane added a comment. Turns out this patch: https://github.com/llvm/llvm-project/commit/2e1e0491b7098fcfe01945e8f62cafe1fcb3cf36 is my problem. The issue has to do deducing a 'local' type in the return type. As a result, we choose to mangle any

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. In D89559#2347642 , @rjmccall wrote: > Mangling more calling conventions when mangling function types in Itanium > (except at the top level) is the right thing to do. There's already a place > to do so in the mangling. We

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added a comment. Mangling more calling conventions when mangling function types in Itanium (except at the top level) is the right thing to do. There's already a place to do so in the mangling. We just haven't done this yet because a lot of those calling convention are supported by

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. Hmm... so I got distracted the last few days with a handful of small SEMA issues that I believe to be solved, so I'm going back to my codegen issues. It seems that my problem is that we don't actually mangle calling-convention in a pointer types. The result is: 64

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. In D89559#2340357 , @rsmith wrote: > In D89559#2339206 , @erichkeane > wrote: > >> Since the work is 99% the same, I think I should start doing that (emitting >> BOTH in the case where

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rsmith added a comment. In D89559#2339206 , @erichkeane wrote: > Since the work is 99% the same, I think I should start doing that (emitting > BOTH in the case where it matches the default member but NOT the default free > function CC) in this review,

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. In D89559#2339192 , @rjmccall wrote: > Yeah, it might be reasonable to just do that on targets where the defaults > are different (just MSVC, I think?) and otherwise preserve the method's > calling convention. I think it is

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added a comment. Yeah, it might be reasonable to just do that on targets where the defaults are different (just MSVC, I think?) and otherwise preserve the method's calling convention. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89559/new/ https://reviews.llvm.org/D89559

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. In D89559#2336335 , @rjmccall wrote: > We can't have it always match, that would require us to reject the conversion > to a bare function-pointer type, which is required by the standard. > > I'm not sure if MSVC's multiple

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added a comment. We can't have it always match, that would require us to reject the conversion to a bare function-pointer type, which is required by the standard. I'm not sure if MSVC's multiple conversions hack is conformant or not, but it's at least more conformant than that.

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; rsmith wrote: > rsmith wrote: > > erichkeane wrote: > > > rjmccall wrote: > > > > ...I made this

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; rsmith wrote: > erichkeane wrote: > > rjmccall wrote: > > > ...I made this comment in my first review, but

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; erichkeane wrote: > rjmccall wrote: > > ...I made this comment in my first review, but Phabricator threw

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. Perhaps a better example: (gdb) p CallOperator->getType()->dump() FunctionProtoType 0x1177edf0 'void (void) __attribute__((vectorcall)) const' const vectorcall `-AutoType 0x1177a0d0 'void' sugar `-BuiltinType 0x11716dc0 'void' CHANGES SINCE LAST ACTION

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added a comment. In D89559#2335557 , @rjmccall wrote: > No, if you put a calling convention on a lambda and then convert it to a > function pointer, you almost certainly want that CC to be honored. > > Is the `AttributedType` still present in

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added a comment. No, if you put a calling convention on a lambda and then convert it to a function pointer, you almost certainly want that CC to be honored. Is the `AttributedType` still present in `CallOperator->getType()`? CHANGES SINCE LAST ACTION

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane updated this revision to Diff 298666. erichkeane marked an inline comment as done. erichkeane added a comment. Calc->get. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89559/new/ https://reviews.llvm.org/D89559 Files: clang/lib/Sema/SemaLambda.cpp

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; rjmccall wrote: > ...I made this comment in my first review, but Phabricator threw it away. > > The

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1268 +calcLambdaConversionFunctionCallConv(Sema , + const FunctionProtoType *CallOpProto) { + CallingConv DefaultFree =

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; ...I made this comment in my first review, but Phabricator threw it away. The attributes let you

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

rjmccall added a comment. I agree that it seems reasonable to preserve an explicit CC from the lambda on the converted function pointer. Comment at: clang/lib/Sema/SemaLambda.cpp:1268 +calcLambdaConversionFunctionCallConv(Sema , + const

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

erichkeane created this revision. erichkeane added reviewers: rsmith, rjmccall. erichkeane requested review of this revision. As mentioned in the defect, the lambda static invoker does not follow the calling convention of the lambda itself, which seems wrong. This patch ensures that the calling