[PATCH] D67028: Use musttail for variadic method thunks when possible

This revision was automatically updated to reflect the committed changes. Closed by commit rL371269: Use musttail for variadic method thunks when possible (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 ___

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/test/CodeGenCXX/ms-thunks-variadic-return.cpp:9 +struct B : virtual A { + // expected-error@+1 2 {{cannot compile this return-adjusting thunk with variadic arguments yet}} + B *clone(const char

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218995. rnk added a comment. - emit an error if we try to clone without a definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cp

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. In D67028#1659881 , @efriedma wrote: > > In your test case, we hit the early return that I linked to, so we don't > > try to clone, and we don't need to emit an error. > > Yes, that's what happens with the Itanium ABI; what happens wi

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218992. rnk added a comment. - merge into thunks.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cpp clang/test/CodeGenCXX/lineta

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. > In your test case, we hit the early return that I linked to, so we don't try > to clone, and we don't need to emit an error. Yes, that's what happens with the Itanium ABI; what happens with the MS ABI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. I think what I said applies to your test case. Basically, in the Itanium C++ ABI, virtual method definitions provide all their thunks as `weak_odr`. We only emit thunks referenced by vtables as an optimization, and they are marked `available_externally`. In your test case,

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. > In the MS ABI, deriving a new class may require the creation of new thunks > for methods that were not overridden, so we can't use the same trick. Yes. MSVC emits an error message "covariant returns with multiple or virtual inheritance not supported for varargs func

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. Oops, meant to actually include the testcase in my last comment: struct V1 { }; struct V2 : virtual V1 { }; struct A { virtual V1 *f(int,...); }; struct B : A { virtual void b(); virtual V2 *f(int,...); }; B b; Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added a comment. In D67028#1653439 , @efriedma wrote: > Do we have test coverage for a variadic, covariant thunk for a function > without a definition? I don't think there's any way for us to actually emit > tha

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. Do we have test coverage for a variadic, covariant thunk for a function without a definition? I don't think there's any way for us to actually emit that, but we should make sure the error message is right. I'm a little concerned that using musttail thunks with the Ita

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk created this revision. rnk added reviewers: rsmith, hans, efriedma. Herald added a subscriber: fedor.sergeev. Herald added a project: clang. This avoids cloning variadic virtual methods when the target supports musttail and the return type is not covariant. I think we never implemented this pr