Use musttail for variadic method thunks when possible

[rnk]

Summary

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.

[efriedma]

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 Itanium ABI will flush out bugs that have been hiding because we have less test coverage on Windows. But it's probably the right thing to do.

clang/lib/CodeGen/CGVTables.cpp

206 ↗

(On Diff #218191)

This is fine, I guess, but how is it related?

[rnk]

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 that, but we should make sure the error message is right.

That appears to have been PR18098, so we have a test for it in thunks.cpp. The way the Itanium C++ ABI works, thunks are always emitted as weak_odr in TUs that provide the implementation, so emitting them with the vtable is just an optimization. Basically, clang never *has* to emit a thunk when the definition isn't present, it can always choose to simply declare it, and that's what we do here: https://github.com/llvm/llvm-project/blob/7c8952197b86790b31731d34d559281840916e1f/clang/lib/CodeGen/CGVTables.cpp#L558

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.

I think my two new tests are redundant with tests in thunks.cpp, so perhaps I should just add some new RUN lines there.

I'm a little concerned that using musttail thunks with the Itanium ABI will flush out bugs that have been hiding because we have less test coverage on Windows. But it's probably the right thing to do.

True, it's a risk. One other thing worth mentioning is that the IR cloning doesn't actually work in the presence of labels-as-values, so we are improving conformance in that extra, extra corner case.

clang/lib/CodeGen/CGVTables.cpp

206 ↗

(On Diff #218191)

The MS ABI implementation of performThisAdjustment returns a pointer that needs to be cast. The covariant test exercises this codepath in the MS ABI, and we didn't do that before. I guess the Itanium one does the cast internally, but the MS ABI impl has this comment:

// Don't need to bitcast back, the call CodeGen will handle this.
return V;

I probably removed the cast when trying to clean up our generated IR.

[efriedma]

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 functions" in cases like the following. It looks like the equivalent isn't implemented in clang? (Is that the llvm_unreachable in this patch?)

[efriedma]

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;

[rnk]

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, 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.

[efriedma]

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?

[rnk]

• merge into thunks.cpp

[rnk]

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 with the MS ABI?

I see, it doesn't work. There's also the case of the MS ABI for ISAs that don't support musttail, although that's less interesting.

[rnk]

• emit an error if we try to clone without a definition

[rnk]

clang/test/CodeGenCXX/ms-thunks-variadic-return.cpp

9 ↗

(On Diff #218995)

I do love the optimism of the CGM.ErrorUnsupported diagnostic: "cannot compile this ${unimplementable_feature} yet".

I suppose it could be done if we standardized a new ABI for variadic virtual methods with pointer-like return values, so that they emit the main implementation under a new symbol name that takes a va_list and then the variadic one thunks to it. =P

[efriedma]

LGTM

cfe/trunk/lib/CodeGen/CGVTables.cpp

CodeGenFunction::GenerateVarArgsThunk(llvm::Function *Fn,
   QualType ResultType = FPT->getReturnType();
 
   // Get the original function
   assert(FnInfo.isVariadic());
   llvm::Type *Ty = CGM.getTypes().GetFunctionType(FnInfo);
   llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
   llvm::Function *BaseFn = cast<llvm::Function>(Callee);
 
+  // Cloning can't work if we don't have a definition. The Microsoft ABI may
+  // require thunks when a definition is not available. Emit an error in these
+  // cases.
+  if (!MD->isDefined()) {
+    CGM.ErrorUnsupported(MD, "return-adjusting thunk with variadic arguments");
+    return Fn;
+  }
+  assert(!BaseFn->isDeclaration() && "cannot clone undefined variadic method");
+
   // Clone to thunk.
   llvm::ValueToValueMapTy VMap;
 
   // We are cloning a function while some Metadata nodes are still unresolved.
   // Ensure that the value mapper does not encounter any of them.
   resolveTopLevelMetadata(BaseFn, VMap);
   llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);
   Fn->replaceAllUsesWith(NewFn);
llvm::BasicBlock::iterator ThisStore =
                I.getOperand(0) == ThisPtr.getPointer();
       });
   assert(ThisStore != EntryBB->end() &&
          "Store of this should be in entry block?");
   // Adjust "this", if necessary.
   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()) {
     // Fix up the returned value, if necessary.
     for (llvm::BasicBlock &BB : *Fn) {
       llvm::Instruction *T = BB.getTerminator();
       if (isa<llvm::ReturnInst>(T)) {
         RValue RV = RValue::get(T->getOperand(0));
void CodeGenFunction::EmitCallAndReturnForThunk(llvm::FunctionCallee Callee,
   const CXXMethodDecl *MD = cast<CXXMethodDecl>(CurGD.getDecl());
 
   // Adjust the 'this' pointer if necessary
   llvm::Value *AdjustedThisPtr =
     Thunk ? CGM.getCXXABI().performThisAdjustment(
                           *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");
     }
     EmitMustTailThunk(CurGD, AdjustedThisPtr, Callee);
     return;
   }
 
llvm::Constant *CodeGenVTables::maybeEmitThunk(GlobalDecl GD,
   // that the return type is meaningless. These thunks can be used to call
   // functions with differing return types, and the caller is required to cast
   // the prototype appropriately to extract the correct value.
   if (IsUnprototyped)
     ThunkFn->addFnAttr("thunk");
 
   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);
   }
 
   setThunkProperties(CGM, TI, ThunkFn, ForVTable, GD);
   return ThunkFn;
 }

cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp

-// 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(...);
 };
 
 class Derived : public virtual Base {
   virtual void VariadicFunction(...);
 };

cfe/trunk/test/CodeGenCXX/ms-thunks-variadic-return.cpp

+// RUN: %clang_cc1 -fno-rtti-data -triple x86_64-windows-msvc -emit-llvm-only %s -verify
+
+// Verify that we error out on this return adjusting thunk that we can't emit.
+
+struct A {
+  virtual A *clone(const char *f, ...) = 0;
+};
+struct B : virtual A {
+  // expected-error@+1 2 {{cannot compile this return-adjusting thunk with variadic arguments yet}}
+  B *clone(const char *f, ...) override;
+};
+struct C : B { int c; };
+C c;

cfe/trunk/test/CodeGenCXX/thunks.cpp

-// 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 doesn't support musttail (yet), so it uses method cloning for
+// variadic thunks. Use it for testing.
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - \
+// RUN:     | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-NONOPT %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - \
+// RUN:     | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-NONOPT,CHECK-DBG %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN:     | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-OPT %s
+
+// Test x86_64, which uses musttail for variadic thunks.
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN:     | FileCheck --check-prefixes=CHECK,CHECK-TAIL,CHECK-OPT %s
+
+// Finally, reuse these tests for the MS ABI.
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN:     | FileCheck --check-prefixes=WIN64 %s
+
 
 namespace Test1 {
 
 // Check that we emit a non-virtual thunk for C::f.
 
 struct A {
   virtual void f();
 };
 
 struct B {
   virtual void f();
 };
 
 struct C : A, B {
   virtual void c();
   
   virtual void f();
 };
 
 // CHECK-LABEL: define void @_ZThn8_N5Test11C1fEv(
 // CHECK-DBG-NOT: dbg.declare
 // CHECK: ret void
+//
+// WIN64-LABEL: define dso_local void @"?f@C@Test1@@UEAAXXZ"(
+// WIN64-LABEL: define linkonce_odr dso_local void @"?f@C@Test1@@W7EAAXXZ"(
+// WIN64: getelementptr i8, i8* {{.*}}, i32 -8
+// WIN64: ret void
 void C::f() { }
 
 }
 
 namespace Test2 {
 
 // Check that we emit a thunk for B::f since it's overriding a virtual base.
 
 struct A {
   virtual void f();
 };
 
 struct B : virtual A {
   virtual void b();
   virtual void f();
 };
 
 // CHECK-LABEL: define void @_ZTv0_n24_N5Test21B1fEv(
 // CHECK-DBG-NOT: dbg.declare
 // CHECK: ret void
 void B::f() { }
 
+// No thunk is used for this case in the MS ABI.
+// WIN64-LABEL: define dso_local void @"?f@B@Test2@@UEAAXXZ"(
+// WIN64-NOT: define {{.*}} void @"?f@B@Test2
+
 }
 
 namespace Test3 {
 
 // Check that we emit a covariant thunk for B::f.
 
 struct V1 { };
 struct V2 : virtual V1 { };
 
 struct A {
   virtual V1 *f();
 };
 
 struct B : A {
   virtual void b();
   
   virtual V2 *f();
 };
 
 // CHECK: define %{{.*}}* @_ZTch0_v0_n24_N5Test31B1fEv(
+// WIN64: define weak_odr dso_local %{{.*}} @"?f@B@Test3@@QEAAPEAUV1@2@XZ"(
 V2 *B::f() { return 0; }
 
 }
 
 namespace Test4 {
 
 // Check that the thunk for 'C::f' has the same visibility as the function itself.
 
struct __attribute__((visibility("protected"))) C : A, B {
   virtual void f();
 };
 
 // CHECK-LABEL: define protected void @_ZThn8_N5Test41C1fEv(
 // CHECK-DBG-NOT: dbg.declare
 // CHECK: ret void
 void C::f() { }
 
+// Visibility doesn't matter on COFF, but whatever. We could add an ELF test
+// mode later.
+// WIN64-LABEL: define protected void @"?f@C@Test4@@UEAAXXZ"(
+// WIN64-LABEL: define linkonce_odr protected void @"?f@C@Test4@@W7EAAXXZ"(
 }
 
 // Check that the thunk gets internal linkage.
 namespace Test4B {
   struct A {
     virtual void f();
   };
 
namespace Test4B {
   void C::f() {}
 
   // Force C::f to be used.
   void f() { 
     C c; 
     c.f();
   }
 }
+// Not sure why this isn't delayed like in Itanium.
+// WIN64-LABEL: define internal void @"?f@C@?A0xAEF74CE7@Test4B@@UEAAXXZ"(
 
 namespace Test5 {
 
 // Check that the thunk for 'B::f' gets the same linkage as the function itself.
 struct A {
   virtual void f();
 };
 
 struct B : virtual A {
   virtual void f() { }
 };
 
 void f(B b) {
   b.f();
 }
+// No thunk in MS ABI in this case.
 }
 
 namespace Test6 {
   struct X {
     X();
     X(const X&);
     X &operator=(const X&);
     ~X();
namespace Test6 {
   };
 
   // CHECK-LABEL: define void @_ZThn16_N5Test66Thunks1fEv
   // CHECK-DBG-NOT: dbg.declare
   // CHECK-NOT: memcpy
   // CHECK: {{call void @_ZN5Test66Thunks1fEv.*sret}}
   // CHECK: ret void
   X Thunks::f() { return X(); }
+
+  // WIN64-LABEL: define linkonce_odr dso_local void @"?f@Thunks@Test6@@WBA@EAA?AUX@2@XZ"({{.*}} sret %{{.*}})
+  // WIN64-NOT: memcpy
+  // WIN64: tail call void @"?f@Thunks@Test6@@UEAA?AUX@2@XZ"({{.*}} sret %{{.*}})
 }
 
 namespace Test7 {
   // PR7188
   struct X {
     X();
     X(const X&);
     X &operator=(const X&);
namespace Test7 {
 
   void D::baz(X, X&, _Complex float, Small, Small&, Large) { }
 
   // CHECK-LABEL: define void @_ZThn8_N5Test71D3bazENS_1XERS1_CfNS_5SmallERS4_NS_5LargeE(
   // CHECK-DBG-NOT: dbg.declare
   // CHECK-NOT: memcpy
   // CHECK: ret void
   void testD() { D d; }
+
+  // MS C++ ABI doesn't use a thunk, so this case isn't interesting.
 }
 
 namespace Test8 {
   struct NonPOD { ~NonPOD(); int x, y, z; };
   struct A { virtual void foo(); };
   struct B { virtual void bar(NonPOD); };
   struct C : A, B { virtual void bar(NonPOD); static void helper(NonPOD); };
 
   // CHECK: define void @_ZN5Test81C6helperENS_6NonPODE([[NONPODTYPE:%.*]]*
   void C::helper(NonPOD var) {}
 
   // CHECK-LABEL: define void @_ZThn8_N5Test81C3barENS_6NonPODE(
   // CHECK-DBG-NOT: dbg.declare
   // CHECK-NOT: load [[NONPODTYPE]], [[NONPODTYPE]]*
   // CHECK-NOT: memcpy
   // CHECK: ret void
   void C::bar(NonPOD var) {}
+
+  // MS C++ ABI doesn't use a thunk, so this case isn't interesting.
 }
 
 // PR7241: Emitting thunks for a method shouldn't require the vtable for
 // that class to be emitted.
 namespace Test9 {
   struct A { virtual ~A() { } };
   struct B : A { virtual void test() const {} };
   struct C : B { C(); ~C(); };
namespace Test11 {
   // CHECK-DBG-NOT: dbg.declare
   // CHECK: ret
 
   //  The return-adjustment thunk required when C::f appears in a vtable
   //  where A is at a zero offset from C.
   // CHECK: define {{.*}} @_ZTch0_v0_n32_N6Test111C1fEv(
   // CHECK-DBG-NOT: dbg.declare
   // CHECK: ret
+
+  // WIN64-LABEL: define dso_local %{{.*}}* @"?f@C@Test11@@UEAAPEAU12@XZ"(i8*
+
+  // WIN64-LABEL: define weak_odr dso_local %{{.*}}* @"?f@C@Test11@@QEAAPEAUA@2@XZ"(i8*
+  // WIN64: call %{{.*}}* @"?f@C@Test11@@UEAAPEAU12@XZ"(i8* %{{.*}})
+  //
+  // Match the vbtable return adjustment.
+  // WIN64: load i32*, i32** %{{[^,]*}}, align 8
+  // WIN64: getelementptr inbounds i32, i32* %{{[^,]*}}, i32 1
+  // WIN64: load i32, i32* %{{[^,]*}}, align 4
 }
 
 // Varargs thunk test.
 namespace Test12 {
   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* C::f(int x, ...) { return this; }
+  C* makeC();
+  C* C::f(int x, ...) { return makeC(); }
 
   // C::f
   // CHECK: define {{.*}} @_ZN6Test121C1fEiz
 
   // Varargs thunk; check that both the this and covariant adjustments
   // are generated.
   // CHECK: define {{.*}} @_ZTchn8_h8_N6Test121C1fEiz
   // CHECK-DBG-NOT: dbg.declare
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 -8
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 8
+
+  // The vtable layout goes:
+  // C vtable in A:
+  // - f impl, no adjustment
+  // C vtable in B:
+  // - f thunk 2, covariant, clone
+  // - f thunk 2, musttail this adjust to impl
+  // FIXME: The weak_odr linkage is probably not necessary and just an artifact
+  // of Itanium ABI details.
+  // WIN64-LABEL: define dso_local {{.*}} @"?f@C@Test12@@UEAAPEAU12@HZZ"(
+  // WIN64: call %{{.*}}* @"?makeC@Test12@@YAPEAUC@1@XZ"()
+  //
+  // This thunk needs return adjustment, clone.
+  // WIN64-LABEL: define weak_odr dso_local {{.*}} @"?f@C@Test12@@W7EAAPEAUB@2@HZZ"(
+  // WIN64: call %{{.*}}* @"?makeC@Test12@@YAPEAUC@1@XZ"()
+  // WIN64: getelementptr inbounds i8, i8* %{{.*}}, i32 8
+  //
+  // Musttail call back to the A implementation after this adjustment from B to A.
+  // WIN64-LABEL: define linkonce_odr dso_local %{{.*}}* @"?f@C@Test12@@W7EAAPEAU12@HZZ"(
+  // WIN64: getelementptr i8, i8* %{{[^,]*}}, i32 -8
+  // WIN64: musttail call {{.*}} @"?f@C@Test12@@UEAAPEAU12@HZZ"(
+  C c;
 }
 
 // PR13832
 namespace Test13 {
   struct B1 {
     virtual B1 &foo1();
   };
   struct Pad1 {
namespace Test13 {
   }
   // CHECK: define {{.*}} @_ZTcvn8_n32_v8_n24_N6Test131D4foo1Ev
   // CHECK-DBG-NOT: dbg.declare
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 -8
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 -32
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 -24
   // CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 8
   // CHECK: ret %"struct.Test13::D"*
+
+  // WIN64-LABEL: define weak_odr dso_local dereferenceable(32) %"struct.Test13::D"* @"?foo1@D@Test13@@$4PPPPPPPE@A@EAAAEAUB1@2@XZ"(
+  //    This adjustment.
+  // WIN64: getelementptr inbounds i8, i8* {{.*}}, i64 -12
+  //    Call implementation.
+  // WIN64: call {{.*}} @"?foo1@D@Test13@@UEAAAEAU12@XZ"(i8* {{.*}})
+  //    Virtual + nonvirtual return adjustment.
+  // WIN64: load i32*, i32** %{{[^,]*}}, align 8
+  // WIN64: getelementptr inbounds i32, i32* %{{[^,]*}}, i32 1
+  // WIN64: load i32, i32* %{{[^,]*}}, align 4
+  // WIN64: getelementptr inbounds i8, i8* %{{[^,]*}}, i32 %{{[^,]*}}
 }
 
 namespace Test14 {
   class A {
     virtual void f();
   };
   class B {
     virtual void f();
namespace Test15 {
   };
   struct C : A, B {
     virtual void c();
     virtual void f(int x, ...);
   };
   void C::c() {}
 
   // C::c
-  // CHECK: declare void @_ZN6Test151C1fEiz
+  // CHECK-CLONE: declare void @_ZN6Test151C1fEiz
   // non-virtual thunk to C::f
-  // CHECK: declare void @_ZThn8_N6Test151C1fEiz
+  // CHECK-CLONE: declare void @_ZThn8_N6Test151C1fEiz
+
+  // If we have musttail, then we emit the thunk as available_externally.
+  // CHECK-TAIL: declare void @_ZN6Test151C1fEiz
+  // CHECK-TAIL: define available_externally void @_ZThn8_N6Test151C1fEiz({{.*}})
+  // CHECK-TAIL: musttail call void (%"struct.Test15::C"*, i32, ...) @_ZN6Test151C1fEiz({{.*}}, ...)
+
+  // MS C++ ABI doesn't use a thunk, so this case isn't interesting.
 }
 
 namespace Test16 {
 struct A {
   virtual ~A();
 };
 struct B {
   virtual void foo();
 };
 struct C : public A, public B {
   void foo() {}
 };
 struct D : public C {
   ~D();
 };
 D::~D() {}
 // CHECK: define linkonce_odr void @_ZThn8_N6Test161C3fooEv({{.*}}) {{.*}} comdat
 // CHECK-DBG-NOT: dbg.declare
 // CHECK: ret void
 }
 
+namespace Test17 {
+class A {
+  virtual void f(const char *, ...);
+};
+class B {
+  virtual void f(const char *, ...);
+};
+class C : A, B {
+  virtual void anchor();
+  void f(const char *, ...) override;
+};
+// Key method and object anchor vtable for Itanium and MSVC.
+void C::anchor() {}
+C c;
+
+// CHECK-CLONE-LABEL: declare void @_ZThn8_N6Test171C1fEPKcz(
+
+// CHECK-TAIL-LABEL: define available_externally void @_ZThn8_N6Test171C1fEPKcz(
+// CHECK-TAIL: getelementptr inbounds i8, i8* %{{.*}}, i64 -8
+// CHECK-TAIL: musttail call {{.*}} @_ZN6Test171C1fEPKcz({{.*}}, ...)
+
+// MSVC-LABEL: define linkonce_odr dso_local void @"?f@C@Test17@@G7EAAXPEBDZZ"
+// MSVC-SAME: (%"class.Test17::C"* %this, i8* %[[ARG:[^,]+]], ...)
+// MSVC: getelementptr i8, i8* %{{.*}}, i32 -8
+// MSVC: musttail call void (%"class.Test17::C"*, i8*, ...) @"?f@C@Test17@@EEAAXPEBDZZ"(%"class.Test17::C"* %{{.*}}, i8* %[[ARG]], ...)
+}
+
 /**** The following has to go at the end of the file ****/
 
 // checking without opt
 // CHECK-NONOPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(
 // CHECK-NONOPT-NOT: comdat
 
 // This is from Test5:
 // CHECK-NONOPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv
 
 // This is from Test10:
 // CHECK-NONOPT-LABEL: define linkonce_odr void @_ZN6Test101C3fooEv
 // CHECK-NONOPT-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv
 
 // Checking with opt
 // CHECK-OPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(%"struct.Test4B::(anonymous namespace)::C"* %this) unnamed_addr #0 align 2
 
 // This is from Test5:
 // CHECK-OPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv
 
 // This is from Test10:
 // CHECK-OPT-LABEL: define linkonce_odr void @_ZN6Test101C3fooEv
 // CHECK-OPT-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv
 
+// This is from Test10:
+// WIN64-LABEL: define linkonce_odr dso_local void @"?foo@C@Test10@@UEAAXXZ"(
+// WIN64-LABEL: define linkonce_odr dso_local void @"?foo@C@Test10@@W7EAAXXZ"(
+
 // CHECK-NONOPT: attributes [[NUW]] = { noinline nounwind optnone uwtable{{.*}} }
 // CHECK-OPT: attributes [[NUW]] = { nounwind uwtable{{.*}} }