mantognini created this revision. Herald added subscribers: cfe-commits, Anastasia, yaxunl. Herald added a project: clang.
This patch does mainly three things: 1. It fixes a false positive error detection in Sema that is similar to D62156 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D62413> and covers a few more things than the above reproducer. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D64569 Files: clang/lib/CodeGen/CGCXXABI.h clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGExprCXX.cpp clang/lib/CodeGen/CodeGenFunction.h clang/lib/CodeGen/ItaniumCXXABI.cpp clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaOverload.cpp clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl
Index: clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl =================================================================== --- /dev/null +++ clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl @@ -0,0 +1,59 @@ +// 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) Index: clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl =================================================================== --- clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl +++ /dev/null @@ -1,14 +0,0 @@ -// 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); Index: clang/lib/Sema/SemaOverload.cpp =================================================================== --- clang/lib/Sema/SemaOverload.cpp +++ clang/lib/Sema/SemaOverload.cpp @@ -5089,12 +5089,10 @@ 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); Index: clang/lib/Sema/SemaDeclCXX.cpp =================================================================== --- clang/lib/Sema/SemaDeclCXX.cpp +++ clang/lib/Sema/SemaDeclCXX.cpp @@ -8425,12 +8425,19 @@ 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; }); - D.setInvalidType(); + if (DiagOccured) + D.setInvalidType(); } // C++0x [class.dtor]p2: Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp =================================================================== --- clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -258,7 +258,8 @@ 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); @@ -1569,7 +1570,8 @@ 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. @@ -1593,7 +1595,7 @@ 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); Index: clang/lib/CodeGen/ItaniumCXXABI.cpp =================================================================== --- clang/lib/CodeGen/ItaniumCXXABI.cpp +++ clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -224,7 +224,8 @@ 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; @@ -1539,7 +1540,8 @@ 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); @@ -1551,7 +1553,8 @@ 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, Index: clang/lib/CodeGen/CodeGenFunction.h =================================================================== --- clang/lib/CodeGen/CodeGenFunction.h +++ clang/lib/CodeGen/CodeGenFunction.h @@ -2554,8 +2554,8 @@ 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, @@ -3674,10 +3674,10 @@ 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, Index: clang/lib/CodeGen/CGExprCXX.cpp =================================================================== --- clang/lib/CodeGen/CGExprCXX.cpp +++ clang/lib/CodeGen/CGExprCXX.cpp @@ -10,12 +10,13 @@ // //===----------------------------------------------------------------------===// -#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" @@ -91,11 +92,36 @@ 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); } Index: clang/lib/CodeGen/CGClass.cpp =================================================================== --- clang/lib/CodeGen/CGClass.cpp +++ clang/lib/CodeGen/CGClass.cpp @@ -2013,7 +2013,7 @@ 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, @@ -2402,10 +2402,10 @@ 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 { Index: clang/lib/CodeGen/CGCXXABI.h =================================================================== --- clang/lib/CodeGen/CGCXXABI.h +++ clang/lib/CodeGen/CGCXXABI.h @@ -378,7 +378,7 @@ 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,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits