[Clang][CodeGen] Fix this argument type for certain destructors

[jacobly]

Summary

With the Microsoft ABI, some destructors need to offset a parameter to
get the derived this pointer, in which case the type of that parameter
should not be a pointer to the derived type.

Fixes #60465

[jacobly]

Added regression test specific to the issue.

[jacobly]

Cleanup and minor fixes.

[aaron.ballman]

Please be sure to add release notes to clang/docs/ReleaseNotes.rst

Things look reasonable to me, but it'd be nice if someone with more Windows ABI experience could take a look as well.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

243

[jacobly]

Address review comments.

[efriedma]

If the parameter attributes are relevant, should we change the CHECK lines in the tests to check for them? At least for some of the tests. Once we fix these tests to enable opaque pointer types, you won't actually be checking for anything.

[jacobly]

Fix bad diff update.

[efriedma]

LGTM

[jacobly]

I was hoping @rnk was available for review, but otherwise this is ready to land and I don't have commit access.

[efriedma]

How do you want to be listed as author in the git commit (Name <email>)?

[jacobly]

Jacob Young <[e-mail]>

[efriedma]

rG674099113533

[efriedma]

Maybe worth cherry-picking to 16 branch? I think someone will need to rebase onto the branch for that, though; there was merge conflict on the microsoft-abi-eh-cleanups.cpp change.

[aaron.ballman]

In D143233#4160206, @efriedma wrote:

Maybe worth cherry-picking to 16 branch? I think someone will need to rebase onto the branch for that, though; there was merge conflict on the microsoft-abi-eh-cleanups.cpp change.

I wouldn't be opposed to picking this onto 16 given that it's fixing an ABI issue, if someone wants to do the work to cherry-pick it.

[jacobly]

In D143233#4161909, @aaron.ballman wrote:

In D143233#4160206, @efriedma wrote:

Maybe worth cherry-picking to 16 branch? I think someone will need to rebase onto the branch for that, though; there was merge conflict on the microsoft-abi-eh-cleanups.cpp change.

I wouldn't be opposed to picking this onto 16 given that it's fixing an ABI issue, if someone wants to do the work to cherry-pick it.

https://github.com/llvm/llvm-project/issues/61124 https://github.com/llvm/llvm-project-release-prs/pull/336

clang/docs/ReleaseNotes.rst

 the flag ``-Wunsafe-buffer-usage`` is enabled.
 
 ``__declspec`` attributes can now be used together with the using keyword. Before
 the attributes on ``__declspec`` was ignored, while now it will be forwarded to the
 point where the alias is used.
 
 Windows Support
 ---------------
+- Fix incorrect alignment attribute on the this parameter of certain
+  non-complete destructors when using the Microsoft ABI.
+  `Issue 60465 <https://github.com/llvm/llvm-project/issues/60465>`_.
 
 AIX Support
 -----------
 
 C Language Changes in Clang
 ---------------------------
 
 C2x Feature Support

clang/lib/CodeGen/CGCXXABI.h

public:
 
   /// Emit destructor variants required by this ABI.
   virtual void EmitCXXDestructors(const CXXDestructorDecl *D) = 0;
 
   /// Get the type of the implicit "this" parameter used by a method. May return
   /// zero if no specific type is applicable, e.g. if the ABI expects the "this"
   /// parameter to point to some artificial offset in a complete object due to
   /// vbases being reordered.
-  virtual const CXXRecordDecl *
-  getThisArgumentTypeForMethod(const CXXMethodDecl *MD) {
-    return MD->getParent();
+  virtual const CXXRecordDecl *getThisArgumentTypeForMethod(GlobalDecl GD) {
+    return cast<CXXMethodDecl>(GD.getDecl())->getParent();
   }
 
   /// Perform ABI-specific "this" argument adjustment required prior to
   /// a call of a virtual function.
   /// The "VirtualCall" argument is true iff the call itself is virtual.
   virtual Address
   adjustThisArgumentForVirtualFunctionCall(CodeGenFunction &CGF, GlobalDecl GD,
                                            Address This, bool VirtualCall) {

clang/lib/CodeGen/CGCall.cpp

 }
 
 const CGFunctionInfo &
 CodeGenTypes::arrangeCXXStructorDeclaration(GlobalDecl GD) {
   auto *MD = cast<CXXMethodDecl>(GD.getDecl());
 
   SmallVector<CanQualType, 16> argTypes;
   SmallVector<FunctionProtoType::ExtParameterInfo, 16> paramInfos;
-  argTypes.push_back(DeriveThisType(MD->getParent(), MD));
+
+  const CXXRecordDecl *ThisType = TheCXXABI.getThisArgumentTypeForMethod(GD);
+  argTypes.push_back(DeriveThisType(ThisType, MD));
 
   bool PassParams = true;
 
   if (auto *CD = dyn_cast<CXXConstructorDecl>(MD)) {
     // A base class inheriting constructor doesn't get forwarded arguments
     // needed to construct a virtual base (or base class thereof).
     if (auto Inherited = CD->getInheritedConstructor())
       PassParams = inheritingCtorHasParams(Inherited, GD.getCtorType());

clang/lib/CodeGen/CGExprCXX.cpp

 struct MemberCallInfo {
   RequiredArgs ReqArgs;
   // Number of prefix arguments for the call. Ignores the `this` pointer.
   unsigned PrefixSize;
 };
 }
 
 static MemberCallInfo
-commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD,
+commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, GlobalDecl GD,
                                   llvm::Value *This, llvm::Value *ImplicitParam,
                                   QualType ImplicitParamTy, const CallExpr *CE,
                                   CallArgList &Args, CallArgList *RtlArgs) {
+  auto *MD = cast<CXXMethodDecl>(GD.getDecl());
+
   assert(CE == nullptr || isa<CXXMemberCallExpr>(CE) ||
          isa<CXXOperatorCallExpr>(CE));
   assert(MD->isInstance() &&
          "Trying to emit a member or operator call expr on a static method!");
 
   // Push the this ptr.
   const CXXRecordDecl *RD =
-      CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
+      CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(GD);
   Args.add(RValue::get(This), CGF.getTypes().DeriveThisType(RD, MD));
 
   // If there is an implicit parameter (e.g. VTT), emit it.
   if (ImplicitParam) {
     Args.add(RValue::get(ImplicitParam), ImplicitParamTy);
   }
 
   const FunctionProtoType *FPT = MD->getType()->castAs<FunctionProtoType>();
RValue CodeGenFunction::EmitCXXDestructorCall(
   if (SrcAS != DstAS) {
     QualType DstTy = DtorDecl->getThisType();
     llvm::Type *NewType = CGM.getTypes().ConvertType(DstTy);
     This = getTargetHooks().performAddrSpaceCast(*this, This, SrcAS, DstAS,
                                                  NewType);
   }
 
   CallArgList Args;
-  commonEmitCXXMemberOrOperatorCall(*this, DtorDecl, This, ImplicitParam,
+  commonEmitCXXMemberOrOperatorCall(*this, Dtor, This, ImplicitParam,
                                     ImplicitParamTy, CE, Args, nullptr);
   return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(Dtor), Callee,
                   ReturnValueSlot(), Args, nullptr, CE && CE == MustTailCall,
                   CE ? CE->getExprLoc() : SourceLocation{});
 }
 
 RValue CodeGenFunction::EmitCXXPseudoDestructorExpr(
                                             const CXXPseudoDestructorExpr *E) {
RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
 
   if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) {
     // This is the MSVC p->Ctor::Ctor(...) extension. We assume that's
     // constructing a new complete object of type Ctor.
     assert(!RtlArgs);
     assert(ReturnValue.isNull() && "Constructor shouldn't have return value");
     CallArgList Args;
     commonEmitCXXMemberOrOperatorCall(
-        *this, Ctor, This.getPointer(*this), /*ImplicitParam=*/nullptr,
+        *this, {Ctor, Ctor_Complete}, This.getPointer(*this),
+        /*ImplicitParam=*/nullptr,
         /*ImplicitParamTy=*/QualType(), CE, Args, nullptr);
 
     EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false,
                            /*Delegating=*/false, This.getAddress(*this), Args,
                            AggValueSlot::DoesNotOverlap, CE->getExprLoc(),
                            /*NewPointerIsChecked=*/false);
     return RValue::get(nullptr);
   }

clang/lib/CodeGen/MicrosoftCXXABI.cpp

void setCXXDestructorDLLStorage(llvm::GlobalValue *GV,
                                   CXXDtorType DT) const override;
 
   llvm::GlobalValue::LinkageTypes
   getCXXDestructorLinkage(GVALinkage Linkage, const CXXDestructorDecl *Dtor,
                           CXXDtorType DT) const override;
 
   void EmitCXXDestructors(const CXXDestructorDecl *D) override;
 
-  const CXXRecordDecl *
-  getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override {
-    if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD)) {
+  const CXXRecordDecl *getThisArgumentTypeForMethod(GlobalDecl GD) override {
+    const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
+
+    if (MD->isVirtual()) {
+      GlobalDecl LookupGD = GD;
+      if (const auto *DD = dyn_cast<CXXDestructorDecl>(MD)) {
+        // Complete dtors take a pointer to the complete object,
+        // thus don't need adjustment.
+        if (GD.getDtorType() == Dtor_Complete)
+          return MD->getParent();
+
+        // There's only Dtor_Deleting in vftable but it shares the this
+        // adjustment with the base one, so look up the deleting one instead.
+        LookupGD = GlobalDecl(DD, Dtor_Deleting);
+      }
       MethodVFTableLocation ML =
-          CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
+          CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
+
       // The vbases might be ordered differently in the final overrider object
       // and the complete object, so the "this" argument may sometimes point to
       // memory that has no particular type (e.g. past the complete object).
       // In this case, we just use a generic pointer type.
       // FIXME: might want to have a more precise type in the non-virtual
       // multiple inheritance case.
       if (ML.VBase || !ML.VFPtrOffset.isZero())
         return nullptr;

clang/test/CodeGenCXX/constructor-destructor-return-this.cpp

 // CHECKIOS5-LABEL: define{{.*}} %class.D* @_ZN1DD1Ev(%class.D* {{[^,]*}} %this)
 
 // CHECKFUCHSIA-LABEL: define{{.*}} %class.D* @_ZN1DC2Ev(%class.D* {{[^,]*}} returned{{[^,]*}} %this, i8** noundef %vtt)
 // CHECKFUCHSIA-LABEL: define{{.*}} %class.D* @_ZN1DC1Ev(%class.D* {{[^,]*}} returned{{[^,]*}} %this)
 // CHECKFUCHSIA-LABEL: define{{.*}} %class.D* @_ZN1DD2Ev(%class.D* {{[^,]*}} returned{{[^,]*}} %this, i8** noundef %vtt)
 // CHECKFUCHSIA-LABEL: define{{.*}} %class.D* @_ZN1DD1Ev(%class.D* {{[^,]*}} returned{{[^,]*}} %this)
 
 // CHECKMS-LABEL: define dso_local x86_thiscallcc noundef %class.D* @"??0D@@QAE@XZ"(%class.D* {{[^,]*}} returned{{[^,]*}} %this, i32 noundef %is_most_derived)
-// CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1D@@UAE@XZ"(%class.D* {{[^,]*}} %this)
+// CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1D@@UAE@XZ"(i8* {{[^,]*}} %this.coerce)
 
 class E {
 public:
   E();
   virtual ~E();
 };
 
 E* gete();

clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp

 // CHECK: %[[d:.*]] = load
 // CHECK: icmp eq %{{.*}} %[[d]], null
 // CHECK: br i1
 //
 // CHECK-NOT: call
 // For MS, we don't add a new vtable slot to the primary vtable for the virtual
 // destructor. Instead we cast to the VDel base class.
 // CHECK-MSABI: bitcast {{.*}} %[[d]]
-// CHECK-MSABI64-NEXT: getelementptr {{.*}}, i64 8
-// CHECK-MSABI32-NEXT: getelementptr {{.*}}, i32 4
-// CHECK-MSABI-NEXT: %[[d:.*]] = bitcast i8*
+// CHECK-MSABI64-NEXT: %[[d:.*]] = getelementptr {{.*}}, i64 8
+// CHECK-MSABI32-NEXT: %[[d:.*]] = getelementptr {{.*}}, i32 4
 //
 // CHECK: %[[VTABLE:.*]] = load
 // CHECK: %[[DTOR:.*]] = load
 //
-// CHECK: call {{void|noundef i8\*|x86_thiscallcc noundef i8\*}} %[[DTOR]](%{{.*}}* {{[^,]*}} %[[d]]
+// CHECK: call {{void|noundef i8\*|x86_thiscallcc noundef i8\*}} %[[DTOR]]({{.*}}* {{[^,]*}} %[[d]]
 // CHECK-MSABI-SAME: , i32 noundef 1)
 // CHECK-NOT: call
 // CHECK: }
 
 struct J {
   J(); // might throw
   void operator delete(J *, std::destroying_delete_t);
 };

clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

 // pointer to call a base destructor from a constructor while unwinding.
 // WIN32-LABEL: define dso_local {{.*}} @"??0C@crash_on_partial_destroy@@QAE@XZ"{{.*}} {
 // WIN32:      cleanuppad
 //
 //        We shouldn't do any vbptr loads, just constant GEPs.
 // WIN32-NOT:  load
 // WIN32:      getelementptr i8, i8* %{{.*}}, i32 4
 // WIN32-NOT:  load
-// WIN32:      bitcast i8* %{{.*}} to %"struct.crash_on_partial_destroy::B"*
 // WIN32:      call x86_thiscallcc void @"??1B@crash_on_partial_destroy@@UAE@XZ"
 //
 // WIN32-NOT:  load
 // WIN32:      bitcast %"struct.crash_on_partial_destroy::C"* %{{.*}} to i8*
 // WIN32-NOT:  load
 // WIN32:      getelementptr inbounds i8, i8* %{{.*}}, i32 4
 // WIN32-NOT:  load
 // WIN32:      bitcast i8* %{{.*}} to %"struct.crash_on_partial_destroy::A"*

clang/test/CodeGenCXX/microsoft-abi-structors.cpp

 struct B {
   virtual ~B();
 };
 struct C : A, B {
   virtual ~C();
 };
 
 C::~C() {
-// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1C@dtor_in_second_nvbase@@UAE@XZ"
-// CHECK:       (%"struct.dtor_in_second_nvbase::C"* {{[^,]*}} %this)
+// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1C@dtor_in_second_nvbase@@UAE@XZ"(i8*{{[^,]*}} %this.coerce)
 //      No this adjustment!
 // CHECK-NOT: getelementptr
 // CHECK:   load %"struct.dtor_in_second_nvbase::C"*, %"struct.dtor_in_second_nvbase::C"** %{{.*}}
 //      Now we this-adjust before calling ~B.
 // CHECK:   bitcast %"struct.dtor_in_second_nvbase::C"* %{{.*}} to i8*
 // CHECK:   getelementptr inbounds i8, i8* %{{.*}}, i32 4
 // CHECK:   bitcast i8* %{{.*}} to %"struct.dtor_in_second_nvbase::B"*
-// CHECK:   call x86_thiscallcc void @"??1B@dtor_in_second_nvbase@@UAE@XZ"
-// CHECK:       (%"struct.dtor_in_second_nvbase::B"* {{[^,]*}} %{{.*}})
+// CHECK:   call x86_thiscallcc void @"??1B@dtor_in_second_nvbase@@UAE@XZ"(%"struct.dtor_in_second_nvbase::B"*{{[^,]*}} %{{.*}})
 // CHECK:   ret void
 }
 
 void foo() {
   C c;
 }
-// DTORS2-LABEL: define linkonce_odr dso_local x86_thiscallcc i8* @"??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z"
-// DTORS2:       (%"struct.dtor_in_second_nvbase::C"* %this, i32 %should_call_delete)
+// DTORS2-LABEL: define linkonce_odr dso_local x86_thiscallcc i8* @"??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z"(i8* %this.coerce, i32 %should_call_delete)
 //      Do an adjustment from B* to C*.
 // DTORS2:   getelementptr i8, i8* %{{.*}}, i32 -4
-// DTORS2:   bitcast i8* %{{.*}} to %"struct.dtor_in_second_nvbase::C"*
 // DTORS2:   %[[CALL:.*]] = tail call x86_thiscallcc i8* @"??_GC@dtor_in_second_nvbase@@UAEPAXI@Z"
 // DTORS2:   ret i8* %[[CALL]]
 }
 
 namespace test2 {
 // Just like dtor_in_second_nvbase, except put that in a vbase of a diamond.
 
 // C's dtor is in the non-primary base.
 struct A { virtual void f(); };
 struct B { virtual ~B(); };
 struct C : A, B { virtual ~C(); int c; };
 
 // Diamond hierarchy, with C as the shared vbase.
 struct D : virtual C { int d; };
 struct E : virtual C { int e; };
 struct F : D, E { ~F(); int f; };
 
 F::~F() {
-// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1F@test2@@UAE@XZ"(%"struct.test2::F"*{{[^,]*}})
+// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1F@test2@@UAE@XZ"(i8*{{[^,]*}})
 //      Do an adjustment from C vbase subobject to F as though F was the
 //      complete type.
 // CHECK:   getelementptr inbounds i8, i8* %{{.*}}, i32 -20
 // CHECK:   bitcast i8* %{{.*}} to %"struct.test2::F"*
 // CHECK:   store %"struct.test2::F"*
 }
 
 void foo() {
   F f;
 }
 // DTORS3-LABEL: define linkonce_odr dso_local x86_thiscallcc void @"??_DF@test2@@QAEXXZ"({{.*}} {{.*}} comdat
 //      Do an adjustment from C* to F*.
 // DTORS3:   getelementptr i8, i8* %{{.*}}, i32 20
-// DTORS3:   bitcast i8* %{{.*}} to %"struct.test2::F"*
 // DTORS3:   call x86_thiscallcc void @"??1F@test2@@UAE@XZ"
 // DTORS3:   ret void
 
 }
 
 namespace constructors {
 
 struct A {

clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp

B::B() {
   // CHECK:   store i32 %[[VTORDISP_VAL]], i32* %[[VTORDISP_PTR]]
 
   // CHECK: ret
 }
 
 B::~B() {
   // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1B@@UAE@XZ"
   // Store initial this:
+  // CHECK:   %[[THIS:.*]] = alloca %struct.B*
   // CHECK:   %[[THIS_ADDR:.*]] = alloca %struct.B*
   // CHECK:   store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR]], align 4
   // Reload and adjust the this parameter:
   // CHECK:   %[[THIS_RELOAD:.*]] = load %struct.B*, %struct.B** %[[THIS_ADDR]]
   // CHECK:   %[[THIS_UNADJ_i8:.*]] = bitcast %struct.B* %[[THIS_RELOAD]] to i8*
   // CHECK:   %[[THIS_ADJ_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_UNADJ_i8]], i32 -8
   // CHECK:   %[[THIS:.*]] = bitcast i8* %[[THIS_ADJ_i8]] to %struct.B*
 
B::~B() {
   foo();  // Avoid the "trivial destructor" optimization.
 
   // CHECK: ret
 
   // CHECK2-LABEL: define linkonce_odr dso_local x86_thiscallcc void @"??_DB@@QAEXXZ"(%struct.B*
   // CHECK2: %[[THIS:.*]] = load %struct.B*, %struct.B** {{.*}}
   // CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8*
   // CHECK2: %[[B_i8:.*]] = getelementptr i8, i8* %[[THIS_i8]], i32 8
-  // CHECK2: %[[B:.*]] = bitcast i8* %[[B_i8]] to %struct.B*
-  // CHECK2: call x86_thiscallcc void @"??1B@@UAE@XZ"(%struct.B* {{[^,]*}} %[[B]])
+  // CHECK2: call x86_thiscallcc void @"??1B@@UAE@XZ"(i8*{{[^,]*}} %[[B_i8]])
   // CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8*
   // CHECK2: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_i8]], i32 8
   // CHECK2: %[[VBASE:.*]] = bitcast i8* %[[VBASE_i8]] to %struct.VBase*
   // CHECK2: call x86_thiscallcc void @"??1VBase@@UAE@XZ"(%struct.VBase* {{[^,]*}} %[[VBASE]])
   // CHECK2: ret
 
   // CHECK2-LABEL: define linkonce_odr dso_local x86_thiscallcc noundef i8* @"??_GB@@UAEPAXI@Z"
+  // CHECK2:   store %struct.B* %{{.*}}, %struct.B** %[[THIS:.*]], align 4
   // CHECK2:   store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR:.*]], align 4
   // CHECK2:   %[[THIS:.*]] = load %struct.B*, %struct.B** %[[THIS_ADDR]]
   // CHECK2:   %[[THIS_PARAM_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8*
   // CHECK2:   %[[THIS_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_PARAM_i8:.*]], i32 -8
   // CHECK2:   %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.B*
   // CHECK2:   call x86_thiscallcc void @"??_DB@@QAEXXZ"(%struct.B* {{[^,]*}} %[[THIS]])
   // ...
   // CHECK2: ret

 // CHECK: %[[OBJ_i8:.*]] = bitcast %struct.B* %[[OBJ]] to i8*
 // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 0
 // CHECK: %[[VBPTR8:.*]] = bitcast i8* %[[VBPTR]] to i32**
 // CHECK: %[[VBTABLE:.*]] = load i32*, i32** %[[VBPTR8]]
 // CHECK: %[[VBENTRY:.*]] = getelementptr inbounds i32, i32* %[[VBTABLE]], i32 1
 // CHECK: %[[VBOFFSET32:.*]] = load i32, i32* %[[VBENTRY]]
 // CHECK: %[[VBOFFSET:.*]] = add nsw i32 0, %[[VBOFFSET32]]
 // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 %[[VBOFFSET]]
-// CHECK: %[[VBASE:.*]] = bitcast i8* %[[VBASE_i8]] to %struct.B*
 //
 // CHECK: %[[OBJ_i8:.*]] = bitcast %struct.B* %[[OBJ]] to i8*
 // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 0
 // CHECK: %[[VBPTR8:.*]] = bitcast i8* %[[VBPTR]] to i32**
 // CHECK: %[[VBTABLE:.*]] = load i32*, i32** %[[VBPTR8]]
 // CHECK: %[[VBENTRY:.*]] = getelementptr inbounds i32, i32* %[[VBTABLE]], i32 1
 // CHECK: %[[VBOFFSET32:.*]] = load i32, i32* %[[VBENTRY]]
 // CHECK: %[[VBOFFSET:.*]] = add nsw i32 0, %[[VBOFFSET32]]
 // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 %[[VBOFFSET]]
-// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to i8* (%struct.B*, i32)***
-// CHECK: %[[VFTABLE:.*]] = load i8* (%struct.B*, i32)**, i8* (%struct.B*, i32)*** %[[VFPTR]]
-// CHECK: %[[VFUN:.*]] = getelementptr inbounds i8* (%struct.B*, i32)*, i8* (%struct.B*, i32)** %[[VFTABLE]], i64 0
-// CHECK: %[[VFUN_VALUE:.*]] = load i8* (%struct.B*, i32)*, i8* (%struct.B*, i32)** %[[VFUN]]
+// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to i8* (i8*, i32)***
+// CHECK: %[[VFTABLE:.*]] = load i8* (i8*, i32)**, i8* (i8*, i32)*** %[[VFPTR]]
+// CHECK: %[[VFUN:.*]] = getelementptr inbounds i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTABLE]], i64 0
+// CHECK: %[[VFUN_VALUE:.*]] = load i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFUN]]
 //
-// CHECK: call x86_thiscallcc noundef i8* %[[VFUN_VALUE]](%struct.B* {{[^,]*}} %[[VBASE]], i32 noundef 1)
+// CHECK: call x86_thiscallcc noundef i8* %[[VFUN_VALUE]](i8* {{[^,]*}} %[[VBASE]], i32 noundef 1)
 // CHECK: ret void
 }
 
 void call_complete_dtor() {
   // CHECK-LABEL: define dso_local void @"?call_complete_dtor@@YAXXZ"
   B b;
   // CHECK: call x86_thiscallcc noundef %struct.B* @"??0B@@QAE@XZ"(%struct.B* {{[^,]*}} %[[B:.*]], i32 noundef 1)
   // CHECK-NOT: getelementptr

 };
 
 struct D : virtual Z, B, C {
   D();
   ~D();
 } d;
 
 D::~D() {
-  // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1D@diamond@@UAE@XZ"(%"struct.diamond::D"*{{.*}})
+  // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1D@diamond@@UAE@XZ"(i8*{{.*}})
   // Store initial this:
+  // CHECK: %[[THIS:.*]] = alloca %"struct.diamond::D"*
   // CHECK: %[[THIS_ADDR:.*]] = alloca %"struct.diamond::D"*
   // CHECK: store %"struct.diamond::D"* %{{.*}}, %"struct.diamond::D"** %[[THIS_ADDR]], align 4
   //
   // Reload and adjust the this parameter:
   // CHECK: %[[THIS_RELOAD:.*]] = load %"struct.diamond::D"*, %"struct.diamond::D"** %[[THIS_ADDR]]
   // CHECK: %[[THIS_UNADJ_i8:.*]] = bitcast %"struct.diamond::D"* %[[THIS_RELOAD]] to i8*
   // CHECK: %[[THIS_ADJ_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_UNADJ_i8]], i32 -24
   // CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_ADJ_i8]] to %"struct.diamond::D"*
   //
   // CHECK: %[[D_i8:.*]] = bitcast %"struct.diamond::D"* %[[THIS]] to i8*
   // CHECK: %[[C_i8:.*]] = getelementptr inbounds i8, i8* %[[D_i8]], i32 4
   // CHECK: %[[C:.*]] = bitcast i8* %[[C_i8]] to %"struct.diamond::C"*
   // CHECK: %[[C_i8:.*]] = bitcast %"struct.diamond::C"* %[[C]] to i8*
-  // CHECK: %[[ARG_i8:.*]] = getelementptr i8, i8* %{{.*}}, i32 16
-  // FIXME: We might consider changing the dtor this parameter type to i8*.
-  // CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::C"*
-  // CHECK: call x86_thiscallcc void @"??1C@diamond@@UAE@XZ"(%"struct.diamond::C"* {{[^,]*}} %[[ARG]])
+  // CHECK: %[[ARG:.*]] = getelementptr i8, i8* %{{.*}}, i32 16
+  // CHECK: call x86_thiscallcc void @"??1C@diamond@@UAE@XZ"(i8*{{[^,]*}} %[[ARG]])
 
   // CHECK: %[[B:.*]] = bitcast %"struct.diamond::D"* %[[THIS]] to %"struct.diamond::B"*
   // CHECK: %[[B_i8:.*]] = bitcast %"struct.diamond::B"* %[[B]] to i8*
-  // CHECK: %[[ARG_i8:.*]] = getelementptr i8, i8* %[[B_i8]], i32 4
-  // CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::B"*
-  // CHECK: call x86_thiscallcc void @"??1B@diamond@@UAE@XZ"(%"struct.diamond::B"* {{[^,]*}} %[[ARG]])
+  // CHECK: %[[ARG:.*]] = getelementptr i8, i8* %[[B_i8]], i32 4
+  // CHECK: call x86_thiscallcc void @"??1B@diamond@@UAE@XZ"(i8*{{[^,]*}} %[[ARG]])
   // CHECK: ret void
 }
 
 }
 
 namespace test2 {
 struct A { A(); };
 struct B : virtual A { B() {} };

 
 // The first non-virtual base doesn't have a vdtor,
 // but "this adjustment" is not needed.
 struct E : D, B, virtual A {
   virtual ~E();
 };
 
 E::~E() {
-  // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1E@test4@@UAE@XZ"(%"struct.test4::E"* {{[^,]*}} %this)
+  // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1E@test4@@UAE@XZ"(i8*{{[^,]*}} %this.coerce)
 
   // In this case "this" points to the most derived class, so no GEPs needed.
+  // CHECK: bitcast i8* %this.coerce
   // CHECK-NOT: getelementptr
   // CHECK-NOT: bitcast
   // CHECK: %[[VFPTR_i8:.*]] = bitcast %"struct.test4::E"* %{{.*}} to i32 (...)***
   // CHECK: store i32 (...)** bitcast ({ [1 x i8*] }* @"??_7E@test4@@6BD@1@@" to i32 (...)**), i32 (...)*** %[[VFPTR_i8]]
   foo(this);
 }
 
 void destroy(E *obj) {
   // CHECK-LABEL: define dso_local void @"?destroy@test4@@YAXPAUE@1@@Z"(%"struct.test4::E"* noundef %obj)
 
   // CHECK-NOT: getelementptr
   // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test4::E"* %[[OBJ]] to i8*
-  // CHECK: %[[B_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4
-  // FIXME: in fact, the call should take i8* and the bitcast is redundant.
-  // CHECK: %[[B_as_E:.*]] = bitcast i8* %[[B_i8]] to %"struct.test4::E"*
+  // CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4
   // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test4::E"* %[[OBJ:.*]] to i8*
   // CHECK: %[[B_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4
-  // CHECK: %[[VPTR:.*]] = bitcast i8* %[[B_i8]] to i8* (%"struct.test4::E"*, i32)***
-  // CHECK: %[[VFTABLE:.*]] = load i8* (%"struct.test4::E"*, i32)**, i8* (%"struct.test4::E"*, i32)*** %[[VPTR]]
-  // CHECK: %[[VFTENTRY:.*]] = getelementptr inbounds i8* (%"struct.test4::E"*, i32)*, i8* (%"struct.test4::E"*, i32)** %[[VFTABLE]], i64 0
-  // CHECK: %[[VFUN:.*]] = load i8* (%"struct.test4::E"*, i32)*, i8* (%"struct.test4::E"*, i32)** %[[VFTENTRY]]
-  // CHECK: call x86_thiscallcc noundef i8* %[[VFUN]](%"struct.test4::E"* {{[^,]*}} %[[B_as_E]], i32 noundef 1)
+  // CHECK: %[[VPTR:.*]] = bitcast i8* %[[B_i8]] to i8* (i8*, i32)***
+  // CHECK: %[[VFTABLE:.*]] = load i8* (i8*, i32)**, i8* (i8*, i32)*** %[[VPTR]]
+  // CHECK: %[[VFTENTRY:.*]] = getelementptr inbounds i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTABLE]], i64 0
+  // CHECK: %[[VFUN:.*]] = load i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTENTRY]]
+  // CHECK: call x86_thiscallcc noundef i8* %[[VFUN]](i8*{{[^,]*}} %[[THIS_i8]], i32 noundef 1)
   delete obj;
 }
 
 }
 
 namespace test5 {
 // PR25370: Don't zero-initialize vbptrs in virtual bases.
 struct A {