[CUDA] Ignore uncallable functions when we check for usual deallocators.

[tra]

Summary

Previously clang considered function variants from both sides of
compilation and that sometimes resulted in picking up wrong deallocation function.

[tra]

@rsmith ping.

[rsmith]

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

551 ↗

(On Diff #164480)

I believe the isOverloadedOperator check here is redundant. The caller already checked that the name is an identifier.

If you want to keep some check of this kind, I'd suggest checking D->getIdentifier() at the start of this function and removing the check here. (Note that CXXMethodDecl is not the only Decl subclass that can have a non-identifier name.)

clang/lib/AST/DeclCXX.cpp

2068–2076 ↗

(On Diff #164480)

I think it would make more sense to put only the 1-parameter functions into Matches, and rename it to PreventedBy or something: then it's the list of single-parameter usual deallocation functions that prevent this one being a usual deallocation function.

[tra]

Addressed Richard's comments.
Moved clang-tidy changes into separate review D52179.

[tra]

Updated assertion message.

[tra]

Renamed last instance of 'Matches' -> 'PreventedBy'.

cfe/trunk/include/clang/AST/DeclCXX.h

public:
   CXXMethodDecl *getDevirtualizedMethod(const Expr *Base, bool IsAppleKext);
 
   const CXXMethodDecl *getDevirtualizedMethod(const Expr *Base,
                                               bool IsAppleKext) const {
     return const_cast<CXXMethodDecl *>(this)->getDevirtualizedMethod(
         Base, IsAppleKext);
   }
 
-  /// Determine whether this is a usual deallocation function
-  /// (C++ [basic.stc.dynamic.deallocation]p2), which is an overloaded
-  /// delete or delete[] operator with a particular signature.
-  bool isUsualDeallocationFunction() const;
+  /// Determine whether this is a usual deallocation function (C++
+  /// [basic.stc.dynamic.deallocation]p2), which is an overloaded delete or
+  /// delete[] operator with a particular signature. Populates \p PreventedBy
+  /// with the declarations of the functions of the same kind if they were the
+  /// reason for this function returning false. This is used by
+  /// Sema::isUsualDeallocationFunction to reconsider the answer based on the
+  /// context.
+  bool isUsualDeallocationFunction(
+      SmallVectorImpl<const FunctionDecl *> &PreventedBy) const;
 
   /// Determine whether this is a copy-assignment operator, regardless
   /// of whether it was declared implicitly or explicitly.
   bool isCopyAssignmentOperator() const;
 
   /// Determine whether this is a move assignment operator.
   bool isMoveAssignmentOperator() const;
 

cfe/trunk/include/clang/Sema/Sema.h

public:
   /// from different modules, and thus an ambiguity error can be downgraded to
   /// an extension warning.
   bool isEquivalentInternalLinkageDeclaration(const NamedDecl *A,
                                               const NamedDecl *B);
   void diagnoseEquivalentInternalLinkageDeclarations(
       SourceLocation Loc, const NamedDecl *D,
       ArrayRef<const NamedDecl *> Equiv);
 
+  bool isUsualDeallocationFunction(const CXXMethodDecl *FD);
+
   bool isCompleteType(SourceLocation Loc, QualType T) {
     return !RequireCompleteTypeImpl(Loc, T, nullptr);
   }
   bool RequireCompleteType(SourceLocation Loc, QualType T,
                            TypeDiagnoser &Diagnoser);
   bool RequireCompleteType(SourceLocation Loc, QualType T,
                            unsigned DiagID);
 

cfe/trunk/lib/AST/DeclCXX.cpp

if (BO->isPtrMemOp()) {
         return DevirtualizedMethod;
     }
   }
 
   // We can't devirtualize the call.
   return nullptr;
 }
 
-bool CXXMethodDecl::isUsualDeallocationFunction() const {
+bool CXXMethodDecl::isUsualDeallocationFunction(
+    SmallVectorImpl<const FunctionDecl *> &PreventedBy) const {
+  assert(PreventedBy.empty() && "PreventedBy is expected to be empty");
   if (getOverloadedOperator() != OO_Delete &&
       getOverloadedOperator() != OO_Array_Delete)
     return false;
 
   // C++ [basic.stc.dynamic.deallocation]p2:
   //   A template instance is never a usual deallocation function,
   //   regardless of its signature.
   if (getPrimaryTemplate())
bool CXXMethodDecl::isUsualDeallocationFunction(
   // In C++17 onwards, all potential usual deallocation functions are actual
   // usual deallocation functions.
   if (Context.getLangOpts().AlignedAllocation)
     return true;
 
   // This function is a usual deallocation function if there are no
   // single-parameter deallocation functions of the same kind.
   DeclContext::lookup_result R = getDeclContext()->lookup(getDeclName());
-  for (DeclContext::lookup_result::iterator I = R.begin(), E = R.end();
-       I != E; ++I) {
-    if (const auto *FD = dyn_cast<FunctionDecl>(*I))
-      if (FD->getNumParams() == 1)
-        return false;
+  bool Result = true;
+  for (const auto *D : R) {
+    if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+      if (FD->getNumParams() == 1) {
+        PreventedBy.push_back(FD);
+        Result = false;
       }
-
-  return true;
+    }
+  }
+  return Result;
 }
 
 bool CXXMethodDecl::isCopyAssignmentOperator() const {
   // C++0x [class.copy]p17:
   //  A user-declared copy assignment operator X::operator= is a non-static
   //  non-template member function of class X with exactly one parameter of
   //  type X, X&, const X&, volatile X& or const volatile X&.
   if (/*operator=*/getOverloadedOperator() != OO_Equal ||

cfe/trunk/lib/Sema/SemaDeclCXX.cpp

if (CheckOperatorNewDeleteTypes(
           SemaRef, FnDecl, SemaRef.Context.VoidTy, ExpectedFirstParamType,
           diag::err_operator_delete_dependent_param_type,
           diag::err_operator_delete_param_type))
     return true;
 
   // C++ P0722:
   //   A destroying operator delete shall be a usual deallocation function.
   if (MD && !MD->getParent()->isDependentContext() &&
-      MD->isDestroyingOperatorDelete() && !MD->isUsualDeallocationFunction()) {
+      MD->isDestroyingOperatorDelete() &&
+      !SemaRef.isUsualDeallocationFunction(MD)) {
     SemaRef.Diag(MD->getLocation(),
                  diag::err_destroying_operator_delete_not_usual);
     return true;
   }
 
   return false;
 }
 

cfe/trunk/lib/Sema/SemaExprCXX.cpp

if (!isa<CXXTemporaryObjectExpr>(Inner) &&
     Result = CXXFunctionalCastExpr::Create(
         Context, ResultType, Expr::getValueKindForType(Ty), TInfo, CK_NoOp,
         Result.get(), /*Path=*/nullptr, Locs.getBegin(), Locs.getEnd());
   }
 
   return Result;
 }
 
+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;
+
+  SmallVector<const FunctionDecl*, 4> PreventedBy;
+  bool Result = Method->isUsualDeallocationFunction(PreventedBy);
+
+  if (Result || !getLangOpts().CUDA || PreventedBy.empty())
+    return Result;
+
+  // In case of CUDA, return true if none of the 1-argument deallocator
+  // functions are actually callable.
+  return llvm::none_of(PreventedBy, [&](const FunctionDecl *FD) {
+    assert(FD->getNumParams() == 1 &&
+           "Only single-operand functions should be in PreventedBy");
+    return IdentifyCUDAPreference(Caller, FD) >= CFP_HostDevice;
+  });
+}
+
 /// Determine whether the given function is a non-placement
 /// deallocation function.
 static bool isNonPlacementDeallocationFunction(Sema &S, FunctionDecl *FD) {
   if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FD))
-    return Method->isUsualDeallocationFunction();
+    return S.isUsualDeallocationFunction(Method);
 
   if (FD->getOverloadedOperator() != OO_Delete &&
       FD->getOverloadedOperator() != OO_Array_Delete)
     return false;
 
   unsigned UsualParams = 1;
 
   if (S.getLangOpts().SizedDeallocation && UsualParams < FD->getNumParams() &&

cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu

+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,DEVICE
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,HOST
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,DEVICE
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,HOST
+
+#include "Inputs/cuda.h"
+extern "C" __host__ void host_fn();
+extern "C" __device__ void dev_fn();
+extern "C" __host__ __device__ void hd_fn();
+
+struct H1D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H2D1 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H2D2 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1H2D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+
+template <typename T>
+__host__ __device__ void test_hd(void *p) {
+  T *t = (T *)p;
+  delete t;
+}
+
+// Make sure we call the right variant of usual deallocator.
+__host__ __device__ void tests_hd(void *t) {
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI4H1D1EvPv
+  // COMMON: call void @_ZN4H1D1dlEPv
+  test_hd<H1D1>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI4H1D2EvPv
+  // DEVICE: call void @_ZN4H1D2dlEPvj(i8* {{.*}}, i32 1)
+  // HOST:   call void @_ZN4H1D2dlEPv(i8* {{.*}})
+  test_hd<H1D2>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI4H2D1EvPv
+  // DEVICE: call void @_ZN4H2D1dlEPv(i8* {{.*}})
+  // HOST:   call void @_ZN4H2D1dlEPvj(i8* %3, i32 1)
+  test_hd<H2D1>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI4H2D2EvPv
+  // COMMON: call void @_ZN4H2D2dlEPvj(i8* {{.*}}, i32 1)
+  test_hd<H2D2>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI6H1D1D2EvPv
+  // COMMON: call void @_ZN6H1D1D2dlEPv(i8* %3)
+  test_hd<H1D1D2>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI6H1H2D1EvPv
+  // COMMON: call void @_ZN6H1H2D1dlEPv(i8* {{.*}})
+  test_hd<H1H2D1>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI6H1H2D2EvPv
+  // DEVICE: call void @_ZN6H1H2D2dlEPvj(i8* {{.*}}, i32 1)
+  // HOST:   call void @_ZN6H1H2D2dlEPv(i8* {{.*}})
+  test_hd<H1H2D2>(t);
+  // COMMON-LABEL: define linkonce_odr void @_Z7test_hdI8H1H2D1D2EvPv
+  // COMMON: call void @_ZN8H1H2D1D2dlEPv(i8* {{.*}})
+  test_hd<H1H2D1D2>(t);
+}
+
+// Make sure we've picked deallocator for the correct side of compilation.
+
+// COMMON-LABEL: define  linkonce_odr void @_ZN4H1D1dlEPv(i8*)
+// DEVICE: call void @dev_fn()
+// HOST:   call void @host_fn()
+
+// DEVICE-LABEL: define  linkonce_odr void @_ZN4H1D2dlEPvj(i8*, i32)
+// DEVICE: call void @dev_fn()
+// HOST-LABEL: define linkonce_odr void @_ZN4H1D2dlEPv(i8*)
+// HOST: call void @host_fn()
+
+// DEVICE-LABEL: define  linkonce_odr void @_ZN4H2D1dlEPv(i8*)
+// DEVICE: call void @dev_fn()
+// HOST-LABEL:  define linkonce_odr void @_ZN4H2D1dlEPvj(i8*, i32)
+// HOST: call void @host_fn()
+
+// COMMON-LABEL: define  linkonce_odr void @_ZN4H2D2dlEPvj(i8*, i32)
+// DEVICE: call void @dev_fn()
+// HOST: call void @host_fn()
+
+// COMMON-LABEL: define  linkonce_odr void @_ZN6H1D1D2dlEPv(i8*)
+// DEVICE: call void @dev_fn()
+// HOST: call void @host_fn()
+
+// COMMON-LABEL: define  linkonce_odr void @_ZN6H1H2D1dlEPv(i8*)
+// DEVICE: call void @dev_fn()
+// HOST: call void @host_fn()
+
+// DEVICE-LABEL: define  linkonce_odr void @_ZN6H1H2D2dlEPvj(i8*, i32)
+// DEVICE: call void @dev_fn()
+// HOST-LABEL: define linkonce_odr void @_ZN6H1H2D2dlEPv(i8*)
+// HOST: call void @host_fn()
+
+// COMMON-LABEL: define  linkonce_odr void @_ZN8H1H2D1D2dlEPv(i8*)
+// DEVICE: call void @dev_fn()
+// HOST: call void @host_fn()

cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu

struct T {
   // expected-note@-1 {{'operator+' declared here}}
 
   void operator-(const T&) {}
   // expected-note@-1 {{'operator-' declared here}}
 
   operator Dummy() { return Dummy(); }
   // expected-note@-1 {{'operator Dummy' declared here}}
 
-  __host__ void operator delete(void*);
-  __device__ void operator delete(void*, size_t);
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void*, __SIZE_TYPE__);
 };
 
 struct U {
-  __device__ void operator delete(void*, size_t) = delete;
+  __device__ void operator delete(void*, __SIZE_TYPE__) = delete;
   __host__ __device__ void operator delete(void*);
 };
 
 __host__ __device__ void T::hd3() {
   host_fn();
   // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 }
 

cfe/trunk/test/SemaCUDA/usual-deallocators.cu

+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+
+#include "Inputs/cuda.h"
+extern __host__ void host_fn();
+extern __device__ void dev_fn();
+extern __host__ __device__ void hd_fn();
+
+struct H1D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct h1D1 {
+  __host__ void operator delete(void *) = delete;
+  // host-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1d1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) = delete;
+  // device-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+};
+
+struct H1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H2D1 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H2D2 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1H2D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+
+template <typename T>
+__host__ __device__ void test_hd(void *p) {
+  T *t = (T *)p;
+  delete t;
+  // host-error@-1 {{attempt to use a deleted function}}
+  // device-error@-2 {{attempt to use a deleted function}}
+}
+
+__host__ __device__ void tests_hd(void *t) {
+  test_hd<H1D1>(t);
+  test_hd<h1D1>(t);
+  // host-note@-1 {{in instantiation of function template specialization 'test_hd<h1D1>' requested here}}
+  test_hd<H1d1>(t);
+  // device-note@-1 {{in instantiation of function template specialization 'test_hd<H1d1>' requested here}}
+  test_hd<H1D2>(t);
+  test_hd<H2D1>(t);
+  test_hd<H2D2>(t);
+  test_hd<H1D1D2>(t);
+  test_hd<H1H2D1>(t);
+  test_hd<H1H2D1>(t);
+  test_hd<H1H2D2>(t);
+  test_hd<H1H2D1D2>(t);
+}