olestrohm created this revision.
olestrohm added reviewers: Anastasia, svenvh, rjmccall.
olestrohm added a project: clang.
Herald added subscribers: ldrumm, yaxunl.
olestrohm requested review of this revision.
Herald added a subscriber: cfe-commits.
This patch aims to add initial support for multiple address spaced destructors.
Currently this parses fine and the destructors are correctly added to the class,
but there is no logic to choose the correct destructor for a variable in a
specific address space.
Currently it always picks the first destructor defined in the source code,
regardless of the
address space of the variable.
This is not aiming to be a complete implementation, as destructors are used in
a very large
number of locations, and so doing all of it in one patch would be very
difficult to review and code.
The goals for this patch is thus to add basic support for destructors, and to
build a framework for
additional work to be done on this front.
Since this feature won't be completed in this patch, I have attempted to ensure
that no C++
functionality is changed, to ensure that the changes will only affect C++ for
OpenCL.
This also has the effect that whenever destructors aren't correctly implemented
it will fallback
to default behaviour, which is exactly the same currently happens.
In summary destructors in C++ for OpenCL are currently unusable for multiple
destructors. With
this patch the feature will be usable in the most common situation, and every
bug that currently
exists that isn't covered by the changes I have made here will continue to be
bugs. The hope is that
this patch therefore is almost entirely a positive, since while it doesn't fix
every bug, it will make the
feature actually work, and will create a base to fix the other bugs as they are
discovered.
This is why it's mostly implemented through default parameters, as that will
allow this change to
remain as small as it is, while also allowing further fixes to come along later.
The feature is also C++ for OpenCL specific to let the fast path remain when
not utilizing the
address spaces, as this new implementation will be slower than the bitfields
that C++ currently
uses, but I hope this can also be optimized in the future if it turns out to be
very slow.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D109609
Files:
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclCXX.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLookup.cpp
clang/test/SemaOpenCLCXX/addrspace-destructors.clcpp
Index: clang/test/SemaOpenCLCXX/addrspace-destructors.clcpp
===================================================================
--- /dev/null
+++ clang/test/SemaOpenCLCXX/addrspace-destructors.clcpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 %s -pedantic -ast-dump | FileCheck %s
+
+// CHECK: CXXDestructorDecl {{.*}} used ~ExactDtor 'void () __private noexcept'
+struct ExactDtor {
+ ~ExactDtor() __private;
+};
+
+// CHECK: CXXDestructorDecl
+// CHECK-NOT: used
+// CHECK-SAME: ~OverloadedDtor 'void () __generic'
+// CHECK: CXXDestructorDecl {{.*}} used ~OverloadedDtor 'void () __private noexcept'
+struct OverloadedDtor {
+ ~OverloadedDtor() __generic;
+ ~OverloadedDtor() __private;
+};
+
+// CHECK: CXXDestructorDecl
+// CHECK-NOT: used
+// CHECK-SAME: ~ImplicitDtor 'void () __global'
+// CHECK: CXXDestructorDecl {{.*}} implicit used ~ImplicitDtor 'void () __private noexcept'
+struct ImplicitDtor {
+ ~ImplicitDtor() __global;
+};
+
+// CHECK: CXXDestructorDecl {{.*}} used ~Templated 'void () __generic noexcept'
+// CHECK: CXXDestructorDecl
+// CHECK-NOT: used
+// CHECK-SAME: ~Templated 'void () __global'
+template <typename T>
+struct Templated {
+ ~Templated() __generic;
+ ~Templated() __global;
+};
+
+// CHECK: CXXDestructorDecl {{.*}} used ~BothUsed 'void () __private noexcept'
+// CHECK: CXXDestructorDecl {{.*}} used ~BothUsed 'void () __global noexcept'
+struct BothUsed {
+ ~BothUsed() __private;
+ ~BothUsed() __global;
+};
+
+__global BothUsed g_inheriting;
+
+kernel void k() {
+ __private ExactDtor exact;
+ __private OverloadedDtor overloaded;
+ __private ImplicitDtor implicit;
+ __private Templated<int> templated;
+ __private BothUsed inheriting;
+}
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3054,13 +3054,10 @@
Functions.append(Operators.begin(), Operators.end());
}
-Sema::SpecialMemberOverloadResult Sema::LookupSpecialMember(CXXRecordDecl *RD,
- CXXSpecialMember SM,
- bool ConstArg,
- bool VolatileArg,
- bool RValueThis,
- bool ConstThis,
- bool VolatileThis) {
+Sema::SpecialMemberOverloadResult
+Sema::LookupSpecialMember(CXXRecordDecl *RD, CXXSpecialMember SM, bool ConstArg,
+ bool VolatileArg, bool RValueThis, bool ConstThis,
+ bool VolatileThis, LangAS AS) {
assert(CanDeclareSpecialMemberFunction(RD) &&
"doing special member lookup into record that isn't fully complete");
RD = RD->getDefinition();
@@ -3082,6 +3079,7 @@
ID.AddInteger(RValueThis);
ID.AddInteger(ConstThis);
ID.AddInteger(VolatileThis);
+ ID.AddInteger((unsigned)AS);
void *InsertPoint;
SpecialMemberOverloadResultEntry *Result =
@@ -3096,12 +3094,11 @@
SpecialMemberCache.InsertNode(Result, InsertPoint);
if (SM == CXXDestructor) {
- if (RD->needsImplicitDestructor()) {
- runWithSufficientStackSpace(RD->getLocation(), [&] {
- DeclareImplicitDestructor(RD);
- });
+ if (RD->needsImplicitDestructor(AS)) {
+ runWithSufficientStackSpace(RD->getLocation(),
+ [&] { DeclareImplicitDestructor(RD, AS); });
}
- CXXDestructorDecl *DD = RD->getDestructor();
+ CXXDestructorDecl *DD = RD->getDestructor(AS);
Result->setMethod(DD);
Result->setKind(DD && !DD->isDeleted()
? SpecialMemberOverloadResult::Success
@@ -3362,10 +3359,11 @@
/// CXXRecordDecl::getDestructor().
///
/// \returns The destructor for this class.
-CXXDestructorDecl *Sema::LookupDestructor(CXXRecordDecl *Class) {
+CXXDestructorDecl *Sema::LookupDestructor(CXXRecordDecl *Class, LangAS AS) {
return cast<CXXDestructorDecl>(LookupSpecialMember(Class, CXXDestructor,
- false, false, false,
- false, false).getMethod());
+ false, false, false, false,
+ false, AS)
+ .getMethod());
}
/// LookupLiteralOperator - Determine which literal operator should be used for
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13216,13 +13216,16 @@
void Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
QualType ResultTy,
- ArrayRef<QualType> Args) {
+ ArrayRef<QualType> Args,
+ LangAS AS = LangAS::Default) {
// Build an exception specification pointing back at this constructor.
FunctionProtoType::ExtProtoInfo EPI = getImplicitMethodEPI(*this, SpecialMem);
- LangAS AS = getDefaultCXXMethodAddrSpace();
+ LangAS DefaultAS = getDefaultCXXMethodAddrSpace();
if (AS != LangAS::Default) {
EPI.TypeQuals.addAddressSpace(AS);
+ } else if (DefaultAS != LangAS::Default) {
+ EPI.TypeQuals.addAddressSpace(DefaultAS);
}
auto QT = Context.getFunctionType(ResultTy, Args, EPI);
@@ -13524,12 +13527,13 @@
DiagnoseUninitializedFields(*this, Constructor);
}
-CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) {
+CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl,
+ LangAS AS) {
// C++ [class.dtor]p2:
// If a class has no user-declared destructor, a destructor is
// declared implicitly. An implicitly-declared destructor is an
// inline public member of its class.
- assert(ClassDecl->needsImplicitDestructor());
+ assert(ClassDecl->needsImplicitDestructor(AS));
DeclaringSpecialMember DSM(*this, ClassDecl, CXXDestructor);
if (DSM.isAlreadyBeingDeclared())
@@ -13563,7 +13567,7 @@
/* Diagnose */ false);
}
- setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None);
+ setupImplicitSpecialMemberType(Destructor, Context.VoidTy, None, AS);
// We don't need to use SpecialMemberIsTrivial here; triviality for
// destructors is easy to compute.
@@ -15451,7 +15455,11 @@
if (VD->isNoDestroy(getASTContext()))
return;
- CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl);
+ LangAS AS = LangAS::Default;
+ if (Context.getLangOpts().OpenCL) {
+ AS = VD->getType().getAddressSpace();
+ }
+ CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl, AS);
// If this is an array, we'll require the destructor during initialization, so
// we can skip over this. We still want to emit exit-time destructor warnings
Index: clang/lib/AST/DeclCXX.cpp
===================================================================
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -126,8 +126,7 @@
SourceLocation IdLoc, IdentifierInfo *Id,
CXXRecordDecl *PrevDecl)
: RecordDecl(K, TK, C, DC, StartLoc, IdLoc, Id, PrevDecl),
- DefinitionData(PrevDecl ? PrevDecl->DefinitionData
- : nullptr) {}
+ DefinitionData(PrevDecl ? PrevDecl->DefinitionData : nullptr) {}
CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK,
DeclContext *DC, SourceLocation StartLoc,
@@ -1884,7 +1883,7 @@
return nullptr;
}
-CXXDestructorDecl *CXXRecordDecl::getDestructor() const {
+CXXDestructorDecl *CXXRecordDecl::getDestructor(LangAS AS) const {
ASTContext &Context = getASTContext();
QualType ClassType = Context.getTypeDeclType(this);
@@ -1894,7 +1893,24 @@
DeclContext::lookup_result R = lookup(Name);
- return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());
+ if (AS == LangAS::Default)
+ return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());
+
+ CXXDestructorDecl *Best = nullptr;
+
+ for (DeclContext::lookup_result::iterator I = R.begin(), E = R.end(); I != E;
+ ++I) {
+ auto *Dtor = dyn_cast<CXXDestructorDecl>(*I);
+ LangAS DtorAS = Dtor->getMethodQualifiers().getAddressSpace();
+ if (Qualifiers::isAddressSpaceSupersetOf(DtorAS, AS) &&
+ (Best == nullptr ||
+ Qualifiers::isAddressSpaceSupersetOf(
+ Best->getMethodQualifiers().getAddressSpace(), DtorAS))) {
+ Best = Dtor;
+ }
+ }
+
+ return Best;
}
static bool isDeclContextInNamespace(const DeclContext *DC) {
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -386,7 +386,7 @@
void setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
QualType ResultTy,
- ArrayRef<QualType> Args);
+ ArrayRef<QualType> Args, LangAS AS);
public:
/// The maximum alignment, same as in llvm::Value. We duplicate them here
@@ -4042,13 +4042,10 @@
LOLR_StringTemplatePack,
};
- SpecialMemberOverloadResult LookupSpecialMember(CXXRecordDecl *D,
- CXXSpecialMember SM,
- bool ConstArg,
- bool VolatileArg,
- bool RValueThis,
- bool ConstThis,
- bool VolatileThis);
+ SpecialMemberOverloadResult
+ LookupSpecialMember(CXXRecordDecl *D, CXXSpecialMember SM, bool ConstArg,
+ bool VolatileArg, bool RValueThis, bool ConstThis,
+ bool VolatileThis, LangAS AS = LangAS::Default);
typedef std::function<void(const TypoCorrection &)> TypoDiagnosticGenerator;
typedef std::function<ExprResult(Sema &, TypoExpr *, TypoCorrection)>
@@ -4144,7 +4141,8 @@
unsigned Quals);
CXXMethodDecl *LookupMovingAssignment(CXXRecordDecl *Class, unsigned Quals,
bool RValueThis, unsigned ThisQuals);
- CXXDestructorDecl *LookupDestructor(CXXRecordDecl *Class);
+ CXXDestructorDecl *LookupDestructor(CXXRecordDecl *Class,
+ LangAS AS = LangAS::Default);
bool checkLiteralOperatorId(const CXXScopeSpec &SS, const UnqualifiedId &Id,
bool IsUDSuffix);
@@ -5970,7 +5968,8 @@
/// destructor will be added.
///
/// \returns The implicitly-declared destructor.
- CXXDestructorDecl *DeclareImplicitDestructor(CXXRecordDecl *ClassDecl);
+ CXXDestructorDecl *DeclareImplicitDestructor(CXXRecordDecl *ClassDecl,
+ LangAS AS = LangAS::Default);
/// DefineImplicitDestructor - Checks for feasibility of
/// defining this destructor as the default destructor.
Index: clang/include/clang/AST/DeclCXX.h
===================================================================
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -15,6 +15,7 @@
#ifndef LLVM_CLANG_AST_DECLCXX_H
#define LLVM_CLANG_AST_DECLCXX_H
+#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTUnresolvedSet.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
@@ -978,7 +979,11 @@
/// Determine whether this class needs an implicit destructor to
/// be lazily declared.
- bool needsImplicitDestructor() const {
+ bool needsImplicitDestructor(LangAS AS = LangAS::Default) const {
+ if (getASTContext().getLangOpts().OpenCL) {
+ return getDestructor(AS) == nullptr;
+ }
+
return !(data().DeclaredSpecialMembers & SMF_Destructor);
}
@@ -1476,7 +1481,7 @@
}
/// Returns the destructor decl for this class.
- CXXDestructorDecl *getDestructor() const;
+ CXXDestructorDecl *getDestructor(LangAS AS = LangAS::Default) const;
/// Returns true if the class destructor, or any implicitly invoked
/// destructors are marked noreturn.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
