[PATCH] D94732: [CUDA] Normalize handling of defauled dtor.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added reviewers: HAPPY, yaxunl.
Herald added a subscriber: bixia.
tra requested review of this revision.
Herald added a project: clang.

Defaulted destructor was treated inconsistently, compared to other compiler-generated functions.

When Sema::IdentifyCUDATarget() got called on just-created dtor which didn't have 
implicit `__host__` `__device__` attributes applied yet, it would treat it as a host function.
That happened to (sometimes) hide the error when dtor referred  to a host-only functions.

Even when we had identified defaulted dtor as a HD function, we still treated it inconsistently during 
selection of usual deallocators, where we did not allow referring to wrong-side functions, while it 
is allowed for other HD functions.

This change brings handling of defaulted dtors in line with other HD functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94732

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCUDA/default-ctor.cu
  clang/test/SemaCUDA/usual-deallocators.cu


Index: clang/test/SemaCUDA/usual-deallocators.cu
===================================================================
--- clang/test/SemaCUDA/usual-deallocators.cu
+++ clang/test/SemaCUDA/usual-deallocators.cu
@@ -93,3 +93,12 @@
   test_hd<H1H2D2>(t);
   test_hd<H1H2D1D2>(t);
 }
+
+// This should produce no errors.  Defaulted destructor should be treated as HD,
+// which allows referencing host-only `operator delete` with a deferred
+// diagnostics that would fire if we ever attempt to codegen it on device..
+struct H {
+  virtual ~H() = default;
+  static void operator delete(void *) {}
+};
+H h;
Index: clang/test/SemaCUDA/default-ctor.cu
===================================================================
--- clang/test/SemaCUDA/default-ctor.cu
+++ clang/test/SemaCUDA/default-ctor.cu
@@ -25,7 +25,7 @@
   InD ind;
   InH inh; // expected-error{{no matching constructor for initialization of 'InH'}}
   InHD inhd;
-  Out out; // expected-error{{no matching constructor for initialization of 'Out'}}
+  Out out;
   OutD outd;
   OutH outh; // expected-error{{no matching constructor for initialization of 'OutH'}}
   OutHD outhd;
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1527,9 +1527,24 @@
 bool Sema::isUsualDeallocationFunction(const CXXMethodDecl *Method) {
   // [CUDA] Ignore this function, if we can't call it.
   const FunctionDecl *Caller = dyn_cast<FunctionDecl>(CurContext);
-  if (getLangOpts().CUDA &&
-      IdentifyCUDAPreference(Caller, Method) <= CFP_WrongSide)
-    return false;
+  if (getLangOpts().CUDA) {
+    auto CallPreference = IdentifyCUDAPreference(Caller, Method);
+    // If it's not callable at all, it's not the right function.
+    if (CallPreference < CFP_WrongSide)
+      return false;
+    if (CallPreference == CFP_WrongSide) {
+      // Maybe. We have to check if there are better alternatives.
+      DeclContext::lookup_result R =
+          Method->getDeclContext()->lookup(Method->getDeclName());
+      for (const auto *D : R) {
+        if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+          if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+            return false;
+        }
+      }
+      // We've found no better variants.
+    }
+  }
 
   SmallVector<const FunctionDecl*, 4> PreventedBy;
   bool Result = Method->isUsualDeallocationFunction(PreventedBy);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15159,7 +15159,15 @@
   // 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
   // though.
-  if (!VD->getType()->isArrayType()) {
+  bool SkipDtorChecks = VD->getType()->isArrayType();
+
+  // CUDA: Skip destructor checks for host-only variables during device-side
+  // compilation
+  SkipDtorChecks |=
+      (LangOpts.CUDAIsDevice && VD->hasGlobalStorage() &&
+       !(VD->hasAttr<CUDADeviceAttr>() || VD->hasAttr<CUDAConstantAttr>() ||
+         VD->hasAttr<CUDASharedAttr>()));
+  if (!SkipDtorChecks) {
     MarkFunctionReferenced(VD->getLocation(), Destructor);
     CheckDestructorAccess(VD->getLocation(), Destructor,
                           PDiag(diag::err_access_dtor_var)
Index: clang/lib/Sema/SemaCUDA.cpp
===================================================================
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -123,7 +123,7 @@
     return CFT_Device;
   } else if (hasAttr<CUDAHostAttr>(D, IgnoreImplicitHDAttr)) {
     return CFT_Host;
-  } else if (D->isImplicit() && !IgnoreImplicitHDAttr) {
+  } else if ((D->isImplicit() || D->isDefaulted()) && !IgnoreImplicitHDAttr) {
     // Some implicit declarations (like intrinsic functions) are not marked.
     // Set the most lenient target on them for maximal flexibility.
     return CFT_HostDevice;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94732.316808.patch
Type: text/x-patch
Size: 4054 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210115/f7ad0b29/attachment.bin>