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 previously because it doesn't handle the covariant
case. But, in the MS ABI, there are some cases where vtable thunks must
be emitted even when the variadic method defintion is not available, so
it looks like we need to implement this. Do it for both ABIs, since it's
a nice size improvement and simplification for Itanium.
Fixes PR43173.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D67028
Files:
clang/lib/CodeGen/CGVTables.cpp
clang/test/CodeGenCXX/linetable-virtual-variadic.cpp
clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
clang/test/CodeGenCXX/thunks-variadic.cpp
clang/test/CodeGenCXX/thunks.cpp
Index: clang/test/CodeGenCXX/thunks.cpp
===================================================================
--- clang/test/CodeGenCXX/thunks.cpp
+++ clang/test/CodeGenCXX/thunks.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT --check-prefix=CHECK-DBG %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=CHECK --check-prefix=CHECK-OPT %s
+// Sparc64 is used because AArch64 and X86_64 would both use musttail.
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT --check-prefix=CHECK-DBG %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=CHECK --check-prefix=CHECK-OPT %s
namespace Test1 {
Index: clang/test/CodeGenCXX/thunks-variadic.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/thunks-variadic.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=MSVC %s
+// RUN: %clang_cc1 %s -triple=x86_64-windows-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=ITANIUM %s
+
+namespace test1 {
+class a {
+ virtual void b(const char *, ...);
+};
+class c {
+ virtual void b(const char *, ...);
+};
+class e : c, a {
+ void b(const char *, ...) override;
+} d;
+
+// MSVC-LABEL: define linkonce_odr dso_local void @"?b@e@test1@@G7EAAXPEBDZZ"
+// MSVC-SAME: (%"class.test1::e"* %this, i8* %[[ARG:[^,]+]], ...)
+// MSVC: getelementptr i8, i8* %{{.*}}, i32 -8
+// MSVC: musttail call void (%"class.test1::e"*, i8*, ...) @"?b@e@test1@@EEAAXPEBDZZ"(%"class.test1::e"* %{{.*}}, i8* %[[ARG]], ...)
+
+// ITANIUM-LABEL: define available_externally dso_local void @_ZThn8_N5test11e1bEPKcz
+// ITANIUM-SAME: (%"class.test1::e"* %this, i8* %{{.*}}, ...)
+// ITANIUM: getelementptr inbounds i8, i8* %{{.*}}, i64 -8
+// ITANIUM: musttail call void (%"class.test1::e"*, i8*, ...) @_ZN5test11e1bEPKcz(%"class.test1::e"* %{{.*}}, i8* %{{.*}}, ...)
+}
Index: clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=MSVC %s
+// RUN: %clang_cc1 %s -triple=x86_64-windows-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=ITANIUM %s
+
+struct A {
+ virtual A *f(int x, ...);
+};
+struct B {
+ virtual B *f(int x, ...);
+};
+struct C : A, B {
+ virtual void c();
+ virtual C *f(int x, ...);
+};
+C *makeC();
+C *C::f(int x, ...) { return makeC(); }
+C c;
+
+// MSVC-LABEL: define dso_local %struct.C* @"?f@C@@UEAAPEAU1@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: call %struct.C* @"?makeC@@YAPEAUC@@XZ"()
+
+// MSVC-LABEL: define weak_odr dso_local %struct.C* @"?f@C@@W7EAAPEAUB@@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: call %struct.C* @"?makeC@@YAPEAUC@@XZ"()
+// MSVC: getelementptr inbounds i8, i8* {{.*}}, i32 8
+// MSVC: ret %struct.C* %7
+
+// MSVC-LABEL: define linkonce_odr dso_local %struct.C* @"?f@C@@W7EAAPEAU1@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: getelementptr i8, i8* %{{[^,]*}}, i32 -8
+// MSVC: musttail call %struct.C* (%struct.C*, i32, ...) @"?f@C@@UEAAPEAU1@HZZ"(%struct.C* %{{[^,]*}}, i32 %{{[^,]*}}, ...)
+
+
+// ITANIUM-LABEL: define dso_local %struct.C* @_ZN1C1fEiz
+// ITANIUM-SAME: (%struct.C* %this, i32 %x, ...)
+// ITANIUM: call %struct.C* @_Z5makeCv()
+
+// ITANIUM-LABEL: define dso_local %struct.C* @_ZTchn8_h8_N1C1fEiz
+// ITANIUM-SAME: (%struct.C* %this, i32 %x, ...)
+// ITANIUM: call %struct.C* @_Z5makeCv()
+// ITANIUM: getelementptr inbounds i8, i8* %{{[^,]*}}, i64 8
Index: clang/test/CodeGenCXX/linetable-virtual-variadic.cpp
===================================================================
--- clang/test/CodeGenCXX/linetable-virtual-variadic.cpp
+++ clang/test/CodeGenCXX/linetable-virtual-variadic.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -debug-info-kind=line-tables-only %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -debug-info-kind=line-directives-only %s -o - | FileCheck %s
+// Sparc64 is used because AArch64 and X86_64 would both use musttail.
+// RUN: %clang_cc1 -triple sparc64-linux-gnu -emit-llvm -debug-info-kind=line-tables-only %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple sparc64-linux-gnu -emit-llvm -debug-info-kind=line-directives-only %s -o - | FileCheck %s
// Crasher for PR22929.
class Base {
virtual void VariadicFunction(...);
Index: clang/lib/CodeGen/CGVTables.cpp
===================================================================
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -165,6 +165,7 @@
llvm::Type *Ty = CGM.getTypes().GetFunctionType(FnInfo);
llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
llvm::Function *BaseFn = cast<llvm::Function>(Callee);
+ assert(!BaseFn->isDeclaration() && "cannot clone undefined variadic method");
// Clone to thunk.
llvm::ValueToValueMapTy VMap;
@@ -201,6 +202,8 @@
Builder.SetInsertPoint(&*ThisStore);
llvm::Value *AdjustedThisPtr =
CGM.getCXXABI().performThisAdjustment(*this, ThisPtr, Thunk.This);
+ AdjustedThisPtr = Builder.CreateBitCast(AdjustedThisPtr,
+ ThisStore->getOperand(0)->getType());
ThisStore->setOperand(0, AdjustedThisPtr);
if (!Thunk.Return.isEmpty()) {
@@ -291,14 +294,17 @@
*this, LoadCXXThisAddress(), Thunk->This)
: LoadCXXThis();
- if (CurFnInfo->usesInAlloca() || IsUnprototyped) {
- // We don't handle return adjusting thunks, because they require us to call
- // the copy constructor. For now, fall through and pretend the return
- // adjustment was empty so we don't crash.
+ // If perfect forwarding is required a variadic method, a method using
+ // inalloca, or an unprototyped thunk, use musttail. Emit an error if this
+ // thunk requires a return adjustment, since that is impossible with musttail.
+ if (CurFnInfo->usesInAlloca() || CurFnInfo->isVariadic() || IsUnprototyped) {
if (Thunk && !Thunk->Return.isEmpty()) {
if (IsUnprototyped)
CGM.ErrorUnsupported(
MD, "return-adjusting thunk with incomplete parameter type");
+ else if (CurFnInfo->isVariadic())
+ llvm_unreachable("shouldn't try to emit musttail return-adjusting "
+ "thunks for variadic functions");
else
CGM.ErrorUnsupported(
MD, "non-trivial argument copy for return-adjusting thunk");
@@ -549,16 +555,32 @@
CGM.SetLLVMFunctionAttributesForDefinition(GD.getDecl(), ThunkFn);
+ // Thunks for variadic methods are special because in general variadic
+ // arguments cannot be perferctly forwarded. In the general case, clang
+ // implements such thunks by cloning the original function body. However, for
+ // thunks with no return adjustment on targets that support musttail, we can
+ // use musttail to perfectly forward the variadic arguments.
+ bool ShouldCloneVarArgs = false;
if (!IsUnprototyped && ThunkFn->isVarArg()) {
- // Varargs thunks are special; we can't just generate a call because
- // we can't copy the varargs. Our implementation is rather
- // expensive/sucky at the moment, so don't generate the thunk unless
- // we have to.
- // FIXME: Do something better here; GenerateVarArgsThunk is extremely ugly.
+ ShouldCloneVarArgs = true;
+ if (TI.Return.isEmpty()) {
+ switch (CGM.getTriple().getArch()) {
+ case llvm::Triple::x86_64:
+ case llvm::Triple::x86:
+ case llvm::Triple::aarch64:
+ ShouldCloneVarArgs = false;
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ if (ShouldCloneVarArgs) {
if (UseAvailableExternallyLinkage)
return ThunkFn;
- ThunkFn = CodeGenFunction(CGM).GenerateVarArgsThunk(ThunkFn, FnInfo, GD,
- TI);
+ ThunkFn =
+ CodeGenFunction(CGM).GenerateVarArgsThunk(ThunkFn, FnInfo, GD, TI);
} else {
// Normal thunk body generation.
CodeGenFunction(CGM).generateThunk(ThunkFn, FnInfo, GD, TI, IsUnprototyped);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
