[OpenCL] Improve destructor support in C++ for OpenCL

[mantognini]

Summary

This patch does mainly three things:

1. It fixes a false positive error detection in Sema that is similar to D62156. The error happens when explicitly calling an overloaded destructor for different address spaces.
2. It selects the correct destructor when multiple overloads for address spaces are available.
3. It inserts the expected address space cast when invoking a destructor, if needed, and therefore fixes a crash due to the unmet assertion in llvm::CastInst::Create.

The following is a reproducer of the three issues:

struct MyType {
  ~MyType() {}
  ~MyType() __constant {}
};

__constant MyType myGlobal{};

kernel void foo() {
  myGlobal.~MyType(); // 1 and 2.
  // 1. error: cannot initialize object parameter of type
  //    '__generic MyType' with an expression of type '__constant MyType'
  // 2. error: no matching member function for call to '~MyType'
}

kernel void bar() {
  // 3. The implicit call to the destructor crashes due to:
  //    Assertion `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
  //    in llvm::CastInst::Create.
  MyType myLocal;
}

The added test depends on D62413 and covers a few more things than the
above reproducer.

[mantognini]

clang/lib/CodeGen/CGClass.cpp

2016

This is the only place where the new parameter has an interesting value. In the ~10 other calls to EmitCXXDestructorCall overloads, the default value of this new parameter is used instead.

clang/lib/CodeGen/CGExprCXX.cpp

96

This new parameter is required in order to call performAddrSpaceCast.

117–118

This is effectively the fix for the third point mentioned in the description.

Alternatively, This = Builder.CreatePointerBitCastOrAddrSpaceCast(This, NewType); seems to work equally well and does not require the extra new parameter.

clang/lib/Sema/SemaDeclCXX.cpp

8428–8439

This is the fix for the first point in the description.

clang/lib/Sema/SemaOverload.cpp

5092–5097

This is the fix for the second point in the description.

clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl

1

This test extends addrspace-ctor.cl, which is therefore deleted.

[Anastasia]

clang/lib/CodeGen/CGExprCXX.cpp

117–118

Yes, I agree just using CreatePointerBitCastOrAddrSpaceCast would be easier.

As far as I remember (@rjmccall can correct me if I am wrong) performAddrSpaceCast was added to workaround the fact that nullptr constant might not be value 0 for some address spaces. However, considering that it's not the first place where some big change has to be done in CodeGen to be able to use the new API I am wondering if it's worth looking at moving this logic to LLVM lowering phases that can map nullptr constant into some arbitrary value. I think the challenge is to find where LLVM assumes that nullptr is always 0. That might not be an easy task.

PS, I am not suggesting to do it for this patch but just an idea to investigate in the future.

[rjmccall]

clang/lib/CodeGen/CGClass.cpp

2016

Arguments that are potentially required for correctness — as opposed to just enabling some optimization — should generally not have defaults. I think you should remove these defaults and require a sensible type to be passed down in all cases.

clang/lib/CodeGen/CGExprCXX.cpp

117–118

I continue to think that it makes sense to set Clang up to be able to handle language-level address spaces that don't exist at the level of LLVM IR.

clang/lib/Sema/SemaDeclCXX.cpp

8428–8439

Can we unify this with the similar check we do elsewhere?

[mantognini]

• Refactored common bits from CheckConstructorDeclarator and CheckDestructorDeclarator.
• Added as many "ThisTy" parameter I could.
• Addressed issue with identifier format in tests (%N to %var.ascast).

[mantognini]

Mind the fact that I've rebased the changes onto a more recent master version. If you look at the diff of v1 against v2 you might see some unrelated changes.

Let me know if there's anything else that I need to change.

clang/lib/CodeGen/CGClass.cpp

1444

I'm not familiar enough with CXXCtorType and such, and would welcome some help for these two FIXMEs.

2016

I've addressed that. I had to add some extra parameter/attributes here and there. Please let me know if anything is can be improved.

clang/lib/CodeGen/CGExprCXX.cpp

96

Now that it's no longer a default parameter. I've group ThisTy with This.

103

Despite no longer having a default parameter, not all call site can provide a meaningful value ATM. That is why this check is still required.

117–118

Alright, I've kept it.

clang/lib/Sema/SemaDeclCXX.cpp

8197

I've refactored that bit out of the two mentioned functions. Mind the fact that it now always check !D.isInvalidType(). I think it makes sense, but let me know if that's not the case.

8200

I've reduced the capture group to the bare minimum. Hopefully, this is consistent with LLVM style. But let me know if I should change this back to [&].

8428–8439

Done.

[rjmccall]

clang/lib/CodeGen/CGClass.cpp

496

I think the rule we want is that the type passed here is the (qualified) object type, but getThisType() will return a pointer type. Consider adding a getThisObjectType() method to CXXMethodDecl which does that computation (and then make getThisType() just wrap that in a PointerType).

1444

Dtor->getThisObjectType() should be right in both of these cases.

2016

Thanks, other than the FIXMEs and the object/pointer mismatches this all looks right; very nicely done.

2368

Same thing about getThisObjectType().

clang/lib/CodeGen/CGExprCXX.cpp

103

Is that resolved with fixing the FIXME?

Please assert that ThisTy->getAsCXXRecordDecl() == Dtor->getParent() to guard against that pointer/object mixup.

375

ThisTy here can be either Base->getType() or Base->getType()->getPointeeType() depending on whether this is an arrow access.

clang/lib/CodeGen/ItaniumCXXABI.cpp

1759

It looks like the only places that pass down a null CE are the two implementations in emitVirtualObjectDelete, which both have a CXXDeleteExpr. You could just have this method take an llvm::PointerUnion<CXXMethodCallExpr*,CXXDeleteExpr*> or something, and then someone else could take advantage of that for better source locations on the virtual call eventually.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

1928

Same point as in the ItaniumCXXABI implementation.

[mantognini]

• Addressed comments

[mantognini]

clang/lib/CodeGen/CGClass.cpp

496

I've done the suggested refactoring, let me know if it needs adjustment.

clang/lib/CodeGen/CGExprCXX.cpp

99

I wasn't 100% sure if this was covered by the next assertion.

103

Yes, indeed. I've therefore simplified the code and added the requested assertion.

375

Thanks for mentioning it. It should be better now.

clang/lib/CodeGen/ItaniumCXXABI.cpp

1759

Alright, makes sense. I've changed the function to take a PointerUnion and removed the FIXMEs.

[rjmccall]

LGTM with one minor request.

clang/lib/AST/DeclCXX.cpp

2267 ↗

(On Diff #210341)

Thanks. Can you extract this implementation out into a static function (i.e. an internal linkage function in this file) that takes an ASTContext& so that getThisType doesn't have to fetch the ASTContext twice?

clang/lib/CodeGen/CGExprCXX.cpp

99

It is.

[mantognini]

• Minor refactoring of getThisObjectType
• Removed unnecessary assertion

[mantognini]

clang/lib/AST/DeclCXX.cpp

2267 ↗

(On Diff #210341)

Sure, no problem.

clang/lib/CodeGen/CGExprCXX.cpp

99

Alright, thanks for the clarification. I've removed it as it doesn't bring anything.

[Anastasia]

LGTM! Thanks!

[Anastasia]

The commit has been reverted due to issues with libc++:

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190715/279833.html

However, without this change dtors are largely not usable in OpenCL due to ICE on wrong IR. I would like to be able to fix this on the release branch since we will publicize C++ for OpenCL as a feature.

I suggest to reopening this review and rework the patch.

[mantognini]

While investigating PR42665, I've noticed that getImplicitObjectArgument doesn't always return a pointer type. For example, when dealing with shared_ptr. In libc++'s test std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp, dumping CE in EmitVirtualDestructorCall shows that the callee is .~Bar (mind the .):

CXXMemberCallExpr 0x29d10a0 'void'
`-MemberExpr 0x29d1070 '<bound member function type>' .~Bar 0x23f5250
  `-CXXMemberCallExpr 0x29d1040 'struct Bar':'struct Bar' lvalue
    `-MemberExpr 0x29d1010 '<bound member function type>' .second 0x2883328
      `-MemberExpr 0x29d0f80 '__compressed_pair<class std::__1::allocator<struct Bar>, struct Bar>':'class std::__1::__compressed_pair<class std::__1::allocator<struct Bar>, struct Bar>' lvalue ->__data_ 0x2896960
        `-CXXThisExpr 0x29d0f70 'class std::__1::__shared_ptr_emplace<struct Bar, class std::__1::allocator<struct Bar> > *' implicit this

However, for CodeGenCXX/virtual-pseudo-destructor-call.cpp in clang test suite, CE is this (mind the -> this time):

CXXMemberCallExpr 0x7787a8 'void'
`-MemberExpr 0x778750 '<bound member function type>' ->~A 0x778288
  `-ImplicitCastExpr 0x778738 'struct A *' <LValueToRValue>
    `-DeclRefExpr 0x778708 'struct A *' lvalue ParmVar 0x778558 'a' 'struct A *'

Tomorrow, I'll try to reduce libc++ failing tests and add it to Clang's suite as part of this review. For now, I've run check-all and it seems that no test are failing anymore (with the patch I'm about to upload).

However, I'm not sure whether getImplicitObjectArgument is expected to have either pointer or non-pointer type. I guess it is, but, John, Ilya, could either of you confirm this?

[mantognini]

Add patch for Bug PR42665.

[mantognini]

clang/lib/CodeGen/ItaniumCXXABI.cpp

1757–1763

This is the fix for PR42665. I've just realised that the same should be done in MicrosoftCXXABI.cpp -- will do that tomorrow.

[rjmccall]

Yes, that's the right fix, although you might also consider adding a getObjectType() to CXXMemberCallExpr.

[mantognini]

• Add minimal regression test for PR42665
• Add CXXMemberCallExpr::getObjectType()

[mantognini]

In D64569#1592059, @rjmccall wrote:

Yes, that's the right fix, although you might also consider adding a getObjectType() to CXXMemberCallExpr.

Thanks, John, it should be better now, but let me know if I can improve something.

[rjmccall]

LGTM

[mantognini]

FYI In order to fix buildbot test failures, I've pushed https://reviews.llvm.org/rG1b2da771f561affe36eb5eb0c7a3d2862c5a5c1c. I'll keep an eye on buildbots for additional fallout.

clang/lib/CodeGen/CGCXXABI.h

public:
   addImplicitConstructorArgs(CodeGenFunction &CGF, const CXXConstructorDecl *D,
                              CXXCtorType Type, bool ForVirtualBase,
                              bool Delegating, CallArgList &Args) = 0;
 
   /// Emit the destructor call.
   virtual void EmitDestructorCall(CodeGenFunction &CGF,
                                   const CXXDestructorDecl *DD, CXXDtorType Type,
                                   bool ForVirtualBase, bool Delegating,
-                                  Address This) = 0;
+                                  Address This, QualType ThisTy) = 0;
 
   /// Emits the VTable definitions required for the given record type.
   virtual void emitVTableDefinitions(CodeGenVTables &CGVT,
                                      const CXXRecordDecl *RD) = 0;
 
   /// Checks if ABI requires extra virtual offset for vtable field.
   virtual bool
   isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF,

clang/lib/CodeGen/CGClass.cpp

struct CallBaseDtor final : EHScopeStack::Cleanup {
 
     void Emit(CodeGenFunction &CGF, Flags flags) override {
       const CXXRecordDecl *DerivedClass =
         cast<CXXMethodDecl>(CGF.CurCodeDecl)->getParent();
 
       const CXXDestructorDecl *D = BaseClass->getDestructor();
       Address Addr =
         CGF.GetAddressOfDirectBaseInCompleteClass(CGF.LoadCXXThisAddress(),
                                                   DerivedClass, BaseClass,
                                                   BaseIsVirtual);
       CGF.EmitCXXDestructorCall(D, Dtor_Base, BaseIsVirtual,
                                 /*Delegating=*/false, Addr);
     }
   };
 
   /// A visitor which checks whether an initializer uses 'this' in a
   /// way which requires the vtable to be properly set.
void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   // The call to operator delete in a deleting destructor happens
   // outside of the function-try-block, which means it's always
   // possible to delegate the destructor body to the complete
   // destructor.  Do so.
   if (DtorType == Dtor_Deleting) {
     RunCleanupsScope DtorEpilogue(*this);
     EnterDtorCleanups(Dtor, Dtor_Deleting);
     if (HaveInsertPoint())
       EmitCXXDestructorCall(Dtor, Dtor_Complete, /*ForVirtualBase=*/false,
                             /*Delegating=*/false, LoadCXXThisAddress());
     return;
   }
 
   // If the body is a function-try-block, enter the try before
   // anything else.
   bool isTryBody = (Body && isa<CXXTryStmt>(Body));
   if (isTryBody)

 void CodeGenFunction::destroyCXXObject(CodeGenFunction &CGF,
                                        Address addr,
                                        QualType type) {
   const RecordType *rtype = type->castAs<RecordType>();
   const CXXRecordDecl *record = cast<CXXRecordDecl>(rtype->getDecl());
   const CXXDestructorDecl *dtor = record->getDestructor();
   assert(!dtor->isTrivial());
   CGF.EmitCXXDestructorCall(dtor, Dtor_Complete, /*for vbase*/ false,
-                            /*Delegating=*/false, addr);
+                            /*Delegating=*/false, addr, type);
 }
 
 void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
                                              CXXCtorType Type,
                                              bool ForVirtualBase,
                                              bool Delegating,
                                              AggValueSlot ThisAVS,
                                              const CXXConstructExpr *E) {
struct CallDelegatingCtorDtor final : EHScopeStack::Cleanup {
 
     CallDelegatingCtorDtor(const CXXDestructorDecl *D, Address Addr,
                            CXXDtorType Type)
       : Dtor(D), Addr(Addr), Type(Type) {}
 
     void Emit(CodeGenFunction &CGF, Flags flags) override {
       CGF.EmitCXXDestructorCall(Dtor, Type, /*ForVirtualBase=*/false,
                                 /*Delegating=*/true, Addr);
     }
   };
 } // end anonymous namespace
 
 void
 CodeGenFunction::EmitDelegatingCXXConstructorCall(const CXXConstructorDecl *Ctor,
                                                   const FunctionArgList &Args) {
   assert(Ctor->isDelegatingConstructor());
 
EHStack.pushCleanup<CallDelegatingCtorDtor>(EHCleanup,
                                                 ClassDecl->getDestructor(),
                                                 ThisPtr, Type);
   }
 }
 
 void CodeGenFunction::EmitCXXDestructorCall(const CXXDestructorDecl *DD,
                                             CXXDtorType Type,
                                             bool ForVirtualBase,
-                                            bool Delegating,
-                                            Address This) {
+                                            bool Delegating, Address This,
+                                            QualType ThisTy) {
   CGM.getCXXABI().EmitDestructorCall(*this, DD, Type, ForVirtualBase,
-                                     Delegating, This);
+                                     Delegating, This, ThisTy);
 }
 
 namespace {
   struct CallLocalDtor final : EHScopeStack::Cleanup {
     const CXXDestructorDecl *Dtor;
     Address Addr;
 
     CallLocalDtor(const CXXDestructorDecl *D, Address Addr)

clang/lib/CodeGen/CGExprCXX.cpp

 //===--- CGExprCXX.cpp - Emit LLVM Code for C++ expressions ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This contains code dealing with code generation of C++ expressions
 //
 //===----------------------------------------------------------------------===//
 
-#include "CodeGenFunction.h"
 #include "CGCUDARuntime.h"
 #include "CGCXXABI.h"
 #include "CGDebugInfo.h"
 #include "CGObjCRuntime.h"
+#include "CodeGenFunction.h"
 #include "ConstantEmitter.h"
+#include "TargetInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/IR/Intrinsics.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 namespace {
RValue CodeGenFunction::EmitCXXMemberOrOperatorCall(
   auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall(
       Args, FPT, CallInfo.ReqArgs, CallInfo.PrefixSize);
   return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr,
                   CE ? CE->getExprLoc() : SourceLocation());
 }
 
 RValue CodeGenFunction::EmitCXXDestructorCall(
     GlobalDecl Dtor, const CGCallee &Callee, llvm::Value *This,
-    llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE) {
+    llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE,
+    QualType ThisTy) {
+  const CXXMethodDecl *DtorDecl = cast<CXXMethodDecl>(Dtor.getDecl());
+
+  // In order to accommodate for the many uses of EmitCXXDestructorCall,
+  // ThisTy argument has a default value. Here, we want to make sure that
+  // we are not missing an unexpected address space cast when no specific
+  // information is passed in. But when given the relevant information,
+  // we ensure a cast is added where necessary.
+  if (ThisTy.isNull()) {
+#ifndef NDEBUG
+    unsigned SrcAS = This->getType()->getPointerAddressSpace();
+    llvm::Type *DstType = CGM.getTypes().ConvertType(DtorDecl->getThisType());
+    unsigned DstAS = DstType->getPointerAddressSpace();
+    assert(SrcAS == DstAS && "Missing expected address space cast");
+#endif
+  } else {
+    LangAS SrcAS = ThisTy.getAddressSpace();
+    LangAS DstAS = DtorDecl->getMethodQualifiers().getAddressSpace();
+    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, cast<CXXMethodDecl>(Dtor.getDecl()),
-                                    This, ImplicitParam, ImplicitParamTy, CE,
-                                    Args, nullptr);
+  commonEmitCXXMemberOrOperatorCall(*this, DtorDecl, This, ImplicitParam,
+                                    ImplicitParamTy, CE, Args, nullptr);
   return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(Dtor), Callee,
                   ReturnValueSlot(), Args);
 }
 
 RValue CodeGenFunction::EmitCXXPseudoDestructorExpr(
                                             const CXXPseudoDestructorExpr *E) {
   QualType DestroyedType = E->getDestroyedType();
   if (DestroyedType.hasStrongOrWeakObjCLifetime()) {
if (UseVirtualCall) {
       else if (!DevirtualizedMethod)
         Callee =
             CGCallee::forDirect(CGM.getAddrOfCXXStructor(GD, FInfo, Ty), GD);
       else {
         Callee = CGCallee::forDirect(CGM.GetAddrOfFunction(GD, Ty), GD);
       }
 
       EmitCXXDestructorCall(GD, Callee, This.getPointer(),
                             /*ImplicitParam=*/nullptr,
                             /*ImplicitParamTy=*/QualType(), nullptr);
     }
     return RValue::get(nullptr);
   }
 
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 

clang/lib/CodeGen/CodeGenFunction.h

void EmitCXXAggrConstructorCall(const CXXConstructorDecl *D,
                                   Address ArrayPtr,
                                   const CXXConstructExpr *E,
                                   bool NewPointerIsChecked,
                                   bool ZeroInitialization = false);
 
   static Destroyer destroyCXXObject;
 
   void EmitCXXDestructorCall(const CXXDestructorDecl *D, CXXDtorType Type,
-                             bool ForVirtualBase, bool Delegating,
-                             Address This);
+                             bool ForVirtualBase, bool Delegating, Address This,
+                             QualType ThisTy = QualType());
 
   void EmitNewArrayInitializer(const CXXNewExpr *E, QualType elementType,
                                llvm::Type *ElementTy, Address NewPtr,
                                llvm::Value *NumElements,
                                llvm::Value *AllocSizeWithoutCookie);
 
   void EmitCXXTemporary(const CXXTemporary *Temporary, QualType TempType,
                         Address Ptr);
public:
 
   RValue
   EmitCXXMemberOrOperatorCall(const CXXMethodDecl *Method,
                               const CGCallee &Callee,
                               ReturnValueSlot ReturnValue, llvm::Value *This,
                               llvm::Value *ImplicitParam,
                               QualType ImplicitParamTy, const CallExpr *E,
                               CallArgList *RtlArgs);
-  RValue EmitCXXDestructorCall(GlobalDecl Dtor,
-                               const CGCallee &Callee,
+  RValue EmitCXXDestructorCall(GlobalDecl Dtor, const CGCallee &Callee,
                                llvm::Value *This, llvm::Value *ImplicitParam,
-                               QualType ImplicitParamTy, const CallExpr *E);
+                               QualType ImplicitParamTy, const CallExpr *E,
+                               QualType ThisTy = QualType());
   RValue EmitCXXMemberCallExpr(const CXXMemberCallExpr *E,
                                ReturnValueSlot ReturnValue);
   RValue EmitCXXMemberOrOperatorMemberCallExpr(const CallExpr *CE,
                                                const CXXMethodDecl *MD,
                                                ReturnValueSlot ReturnValue,
                                                bool HasQualifier,
                                                NestedNameSpecifier *Qualifier,
                                                bool IsArrow, const Expr *Base);

clang/lib/CodeGen/ItaniumCXXABI.cpp

public:
 
   AddedStructorArgs
   addImplicitConstructorArgs(CodeGenFunction &CGF, const CXXConstructorDecl *D,
                              CXXCtorType Type, bool ForVirtualBase,
                              bool Delegating, CallArgList &Args) override;
 
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
                           CXXDtorType Type, bool ForVirtualBase,
-                          bool Delegating, Address This) override;
+                          bool Delegating, Address This,
+                          QualType ThisTy) override;
 
   void emitVTableDefinitions(CodeGenVTables &CGVT,
                              const CXXRecordDecl *RD) override;
 
   bool isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF,
                                            CodeGenFunction::VPtr Vptr) override;
 
   bool doStructorsInitializeVPtrs(const CXXRecordDecl *VTableClass) override {
CGCXXABI::AddedStructorArgs ItaniumCXXABI::addImplicitConstructorArgs(
   QualType VTTTy = getContext().getPointerType(getContext().VoidPtrTy);
   Args.insert(Args.begin() + 1, CallArg(RValue::get(VTT), VTTTy));
   return AddedStructorArgs::prefix(1);  // Added one arg.
 }
 
 void ItaniumCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
                                        const CXXDestructorDecl *DD,
                                        CXXDtorType Type, bool ForVirtualBase,
-                                       bool Delegating, Address This) {
+                                       bool Delegating, Address This,
+                                       QualType ThisTy) {
   GlobalDecl GD(DD, Type);
   llvm::Value *VTT = CGF.GetVTTParameter(GD, ForVirtualBase, Delegating);
   QualType VTTTy = getContext().getPointerType(getContext().VoidPtrTy);
 
   CGCallee Callee;
   if (getContext().getLangOpts().AppleKext &&
       Type != Dtor_Base && DD->isVirtual())
     Callee = CGF.BuildAppleKextVirtualDestructorCall(DD, Type, DD->getParent());
   else
     Callee = CGCallee::forDirect(CGM.getAddrOfCXXStructor(GD), GD);
 
-  CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(), VTT, VTTTy, nullptr);
+  CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+                            ThisTy);
 }
 
 void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
                                           const CXXRecordDecl *RD) {
   llvm::GlobalVariable *VTable = getAddrOfVTable(RD, CharUnits());
   if (VTable->hasInitializer())
     return;
 
llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall(
   GlobalDecl GD(Dtor, DtorType);
   const CGFunctionInfo *FInfo =
       &CGM.getTypes().arrangeCXXStructorDeclaration(GD);
   llvm::FunctionType *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
   CGCallee Callee = CGCallee::forVirtual(CE, GD, This, Ty);
 
   CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(), nullptr, QualType(),
                             nullptr);
   return nullptr;
 }
 
 void ItaniumCXXABI::emitVirtualInheritanceTables(const CXXRecordDecl *RD) {
   CodeGenVTables &VTables = CGM.getVTables();
   llvm::GlobalVariable *VTT = VTables.GetAddrOfVTT(RD);
   VTables.EmitVTTDefinition(VTT, CGM.getVTableLinkage(RD), RD);
 }
 
 bool ItaniumCXXABI::canSpeculativelyEmitVTableAsBaseClass(
     const CXXRecordDecl *RD) const {
   // We don't emit available_externally vtables if we are in -fapple-kext mode
   // because kext mode does not permit devirtualization.
   if (CGM.getLangOpts().AppleKext)
     return false;

clang/lib/CodeGen/MicrosoftCXXABI.cpp

public:
 
   AddedStructorArgs
   addImplicitConstructorArgs(CodeGenFunction &CGF, const CXXConstructorDecl *D,
                              CXXCtorType Type, bool ForVirtualBase,
                              bool Delegating, CallArgList &Args) override;
 
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
                           CXXDtorType Type, bool ForVirtualBase,
-                          bool Delegating, Address This) override;
+                          bool Delegating, Address This,
+                          QualType ThisTy) override;
 
   void emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD,
                               llvm::GlobalVariable *VTable);
 
   void emitVTableDefinitions(CodeGenVTables &CGVT,
                              const CXXRecordDecl *RD) override;
 
   bool isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF,
CGCXXABI::AddedStructorArgs MicrosoftCXXABI::addImplicitConstructorArgs(
   }
   Args.add(RV, getContext().IntTy);
   return AddedStructorArgs::suffix(1);
 }
 
 void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
                                          const CXXDestructorDecl *DD,
                                          CXXDtorType Type, bool ForVirtualBase,
-                                         bool Delegating, Address This) {
+                                         bool Delegating, Address This,
+                                         QualType ThisTy) {
   // Use the base destructor variant in place of the complete destructor variant
   // if the class has no virtual bases. This effectively implements some of the
   // -mconstructor-aliases optimization, but as part of the MS C++ ABI.
   if (Type == Dtor_Complete && DD->getParent()->getNumVBases() == 0)
     Type = Dtor_Base;
 
   GlobalDecl GD(DD, Type);
   CGCallee Callee = CGCallee::forDirect(CGM.getAddrOfCXXStructor(GD), GD);
 
   if (DD->isVirtual()) {
     assert(Type != CXXDtorType::Dtor_Deleting &&
            "The deleting destructor should only be called via a virtual call");
     This = adjustThisArgumentForVirtualFunctionCall(CGF, GlobalDecl(DD, Type),
                                                     This, false);
   }
 
   llvm::BasicBlock *BaseDtorEndBB = nullptr;
   if (ForVirtualBase && isa<CXXConstructorDecl>(CGF.CurCodeDecl)) {
     BaseDtorEndBB = EmitDtorCompleteObjectHandler(CGF);
   }
 
   CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(),
                             /*ImplicitParam=*/nullptr,
-                            /*ImplicitParamTy=*/QualType(), nullptr);
+                            /*ImplicitParamTy=*/QualType(), nullptr, ThisTy);
   if (BaseDtorEndBB) {
     // Complete object handler should continue to be the remaining
     CGF.Builder.CreateBr(BaseDtorEndBB);
     CGF.EmitBlock(BaseDtorEndBB);
   }
 }
 
 void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info,
llvm::Value *ImplicitParam = llvm::ConstantInt::get(
       DtorType == Dtor_Deleting);
 
   This = adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
   RValue RV = CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(),
                                         ImplicitParam, Context.IntTy, CE);
   return RV.getScalarVal();
 }
 
 const VBTableGlobals &
 MicrosoftCXXABI::enumerateVBTables(const CXXRecordDecl *RD) {
   // At this layer, we can key the cache off of a single class, which is much
   // easier than caching each vbtable individually.
   llvm::DenseMap<const CXXRecordDecl*, VBTableGlobals>::iterator Entry;
   bool Added;
   std::tie(Entry, Added) =
       VBTablesMap.insert(std::make_pair(RD, VBTableGlobals()));
   VBTableGlobals &VBGlobals = Entry->second;

clang/lib/Sema/SemaDeclCXX.cpp

void Sema::ActOnFinishDelayedCXXMethodDeclaration(Scope *S, Decl *MethodD) {
   if (!Method->isInvalidDecl())
     CheckCXXDefaultArguments(Method);
 }
 
 /// CheckConstructorDeclarator - Called by ActOnDeclarator to check
 /// the well-formedness of the constructor declarator @p D with type @p
 /// R. If there are any errors in the declarator, this routine will
 /// emit diagnostics and set the invalid bit to true.  In any case, the type
 /// will be updated to reflect a well-formed type for the constructor and
 /// returned.
 QualType Sema::CheckConstructorDeclarator(Declarator &D, QualType R,
                                           StorageClass &SC) {
   bool isVirtual = D.getDeclSpec().isVirtualSpecified();
 
   // C++ [class.ctor]p3:
   //   A constructor shall not be virtual (10.3) or static (9.4). A
   //   constructor can be invoked for a const, volatile or const
   //   volatile object. A constructor shall not be declared const,
   //   volatile, or const volatile (9.3.2).
   if (isVirtual) {
else if (unsigned TypeQuals = D.getDeclSpec().getTypeQualifiers()) {
                                 D.getDeclSpec().getRestrictSpecLoc(),
                                 D.getDeclSpec().getAtomicSpecLoc());
       D.setInvalidType();
     }
   }
 
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
   if (FTI.hasMethodTypeQualifiers() && !D.isInvalidType()) {
+    bool DiagOccured = false;
     FTI.MethodQualifiers->forEachQualifier(
         [&](DeclSpec::TQ TypeQual, StringRef QualName, SourceLocation SL) {
+          // This diagnostic should be emitted on any qualifier except an addr
+          // space qualifier. However, forEachQualifier currently doesn't visit
+          // addr space qualifiers, so there's no way to write this condition
+          // right now; we just diagnose on everything.
           Diag(SL, diag::err_invalid_qualified_destructor)
               << QualName << SourceRange(SL);
+          DiagOccured = true;
         });
+    if (DiagOccured)
       D.setInvalidType();
   }
 
   // C++0x [class.dtor]p2:
   //   A destructor shall not be declared with a ref-qualifier.
   if (FTI.hasRefQualifier()) {
     Diag(FTI.getRefQualifierLoc(), diag::err_ref_qualifier_destructor)
       << FTI.RefQualifierIsLValueRef
       << FixItHint::CreateRemoval(FTI.getRefQualifierLoc());

clang/lib/Sema/SemaOverload.cpp

 static ImplicitConversionSequence
 TryObjectArgumentInitialization(Sema &S, SourceLocation Loc, QualType FromType,
                                 Expr::Classification FromClassification,
                                 CXXMethodDecl *Method,
                                 CXXRecordDecl *ActingContext) {
   QualType ClassType = S.Context.getTypeDeclType(ActingContext);
   // [class.dtor]p2: A destructor can be invoked for a const, volatile or
   //                 const volatile object.
-  Qualifiers Quals;
+  Qualifiers Quals = Method->getMethodQualifiers();
   if (isa<CXXDestructorDecl>(Method)) {
     Quals.addConst();
     Quals.addVolatile();
-  } else {
-    Quals = Method->getMethodQualifiers();
   }
 
   QualType ImplicitParamType = S.Context.getQualifiedType(ClassType, Quals);
 
   // Set up the conversion sequence as a "bad" conversion, to allow us
   // to exit early.
   ImplicitConversionSequence ICS;
 

clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl

-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
-
-struct MyType {
-  MyType(int i) : i(i) {}
-  MyType(int i) __constant : i(i) {}
-  int i;
-};
-
-//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const1, i32 1)
-__constant MyType const1 = 1;
-//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const2, i32 2)
-__constant MyType const2(2);
-//CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*), i32 1)
-MyType glob(1);

clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl

+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-DEFINITIONS
+
+// This test ensures the proper address spaces and address space cast are used
+// for constructors, member functions and destructors.
+// See also atexit.cl and global_init.cl for other specific tests.
+
+// CHECK: %struct.MyType = type { i32 }
+struct MyType {
+  MyType(int i) : i(i) {}
+  MyType(int i) __constant : i(i) {}
+  ~MyType() {}
+  ~MyType() __constant {}
+  int bar() { return i + 2; }
+  int bar() __constant { return i + 1; }
+  int i;
+};
+
+// CHECK: @const1 = addrspace(2) global %struct.MyType zeroinitializer
+__constant MyType const1 = 1;
+// CHECK: @const2 = addrspace(2) global %struct.MyType zeroinitializer
+__constant MyType const2(2);
+// CHECK: @glob = addrspace(1) global %struct.MyType zeroinitializer
+MyType glob(1);
+
+// CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const1, i32 1)
+// CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const2, i32 2)
+// CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*), i32 1)
+
+// CHECK-LABEL: define spir_kernel void @fooGlobal()
+kernel void fooGlobal() {
+  // CHECK: call i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*))
+  glob.bar();
+  // CHECK: call i32 @_ZNU3AS26MyType3barEv(%struct.MyType addrspace(2)* @const1)
+  const1.bar();
+  // CHECK: call void @_ZNU3AS26MyTypeD1Ev(%struct.MyType addrspace(2)* @const1)
+  const1.~MyType();
+}
+
+// CHECK-LABEL: define spir_kernel void @fooLocal()
+kernel void fooLocal() {
+  // CHECK: [[VAR:%.*]] = alloca %struct.MyType
+  // CHECK: [[REG:%[0-9]+]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* [[REG]], i32 3)
+  MyType myLocal(3);
+  // CHECK: [[REG:%[0-9]+]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* [[REG]])
+  myLocal.bar();
+  // CHECK: [[REG:%[0-9]+]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call void @_ZNU3AS46MyTypeD1Ev(%struct.MyType addrspace(4)* [[REG]])
+}
+
+// Ensure all members are defined for all the required address spaces.
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* %this, i32 %i)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* %this, i32 %i)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS26MyTypeD1Ev(%struct.MyType addrspace(2)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS46MyTypeD1Ev(%struct.MyType addrspace(4)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr i32 @_ZNU3AS26MyType3barEv(%struct.MyType addrspace(2)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* %this)