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

2021-01-21 Thread Artem Belevich via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG127091bfd5ed: [CUDA] Normalize handling of defauled dtor. 
(authored by tra).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCUDA/usual-deallocators.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(t);
   test_hd(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/CodeGenCUDA/usual-deallocators.cu
===
--- clang/test/CodeGenCUDA/usual-deallocators.cu
+++ clang/test/CodeGenCUDA/usual-deallocators.cu
@@ -12,6 +12,19 @@
 extern "C" __device__ void dev_fn();
 extern "C" __host__ __device__ void hd_fn();
 
+// Destructors are handled a bit differently, compared to regular functions.
+// Make sure we do trigger kernel generation on the GPU side even if it's only
+// referenced by the destructor.
+template __global__ void f(T) {}
+template struct A {
+  ~A() { f<<<1, 1>>>(T()); }
+};
+
+// HOST-LABEL: @a
+A a;
+// HOST-LABEL: define linkonce_odr void @_ZN1AIiED1Ev
+// search further down for the deice-side checks for @_Z1fIiEvT_
+
 struct H1D1 {
   __host__ void operator delete(void *) { host_fn(); };
   __device__ void operator delete(void *) { dev_fn(); };
@@ -95,6 +108,9 @@
   test_hd(t);
 }
 
+// Make sure that we've generated the kernel used by A::~A.
+// DEVICE-LABEL: define dso_local void @_Z1fIiEvT_
+
 // Make sure we've picked deallocator for the correct side of compilation.
 
 // COMMON-LABEL: define  linkonce_odr void @_ZN4H1D1dlEPv(i8* %0)
@@ -131,3 +147,5 @@
 // COMMON-LABEL: define  linkonce_odr void @_ZN8H1H2D1D2dlEPv(i8* %0)
 // DEVICE: call void @dev_fn()
 // HOST: call void @host_fn()
+
+// DEVICE: !0 = !{void (i32)* @_Z1fIiEvT_, !"kernel", i32 1}
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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector PreventedBy;
   bool Result = Method->isUsualDeallocationFunction(PreventedBy);
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -123,7 +123,8 @@
 return CFT_Device;
   } else if (hasAttr(D, IgnoreImplicitHDAttr)) {
 return CFT_Host;
-  } else if (D->isImplicit() && !IgnoreImplicitHDAttr) {
+  } else if ((D->isImplicit() || !D->isUserProvided()) &&
+ !IgnoreImplicitHDAttr) {
 // Some implicit declarations (like intrinsic functions) are not marked.
 // Set the most lenient target on them for maximal flexibility.
 return CFT_HostDevice;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-20 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 318052.
tra added a comment.

Added a test for the corner case Richard has pointed out in the comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCUDA/usual-deallocators.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(t);
   test_hd(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/CodeGenCUDA/usual-deallocators.cu
===
--- clang/test/CodeGenCUDA/usual-deallocators.cu
+++ clang/test/CodeGenCUDA/usual-deallocators.cu
@@ -12,6 +12,19 @@
 extern "C" __device__ void dev_fn();
 extern "C" __host__ __device__ void hd_fn();
 
+// Destructors are handled a bit differently, compared to regular functions.
+// Make sure we do trigger kernel generation on the GPU side even if it's only
+// referenced by the destructor.
+template __global__ void f(T) {}
+template struct A {
+  ~A() { f<<<1, 1>>>(T()); }
+};
+
+// HOST-LABEL: @a
+A a;
+// HOST-LABEL: define linkonce_odr void @_ZN1AIiED1Ev
+// search further down for the deice-side checks for @_Z1fIiEvT_
+
 struct H1D1 {
   __host__ void operator delete(void *) { host_fn(); };
   __device__ void operator delete(void *) { dev_fn(); };
@@ -95,6 +108,9 @@
   test_hd(t);
 }
 
+// Make sure that we've generated the kernel used by A::~A.
+// DEVICE-LABEL: define dso_local void @_Z1fIiEvT_
+
 // Make sure we've picked deallocator for the correct side of compilation.
 
 // COMMON-LABEL: define  linkonce_odr void @_ZN4H1D1dlEPv(i8* %0)
@@ -131,3 +147,5 @@
 // COMMON-LABEL: define  linkonce_odr void @_ZN8H1H2D1D2dlEPv(i8* %0)
 // DEVICE: call void @dev_fn()
 // HOST: call void @host_fn()
+
+// DEVICE: !0 = !{void (i32)* @_Z1fIiEvT_, !"kernel", i32 1}
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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector PreventedBy;
   bool Result = Method->isUsualDeallocationFunction(PreventedBy);
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -123,7 +123,8 @@
 return CFT_Device;
   } else if (hasAttr(D, IgnoreImplicitHDAttr)) {
 return CFT_Host;
-  } else if (D->isImplicit() && !IgnoreImplicitHDAttr) {
+  } else if ((D->isImplicit() || !D->isUserProvided()) &&
+ !IgnoreImplicitHDAttr) {
 // Some implicit declarations (like intrinsic functions) are not marked.
 // Set the most lenient target on them for maximal flexibility.
 return CFT_HostDevice;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-20 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 318018.
tra added a comment.

Removed unneeded changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  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(t);
   test_hd(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/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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector PreventedBy;
   bool Result = Method->isUsualDeallocationFunction(PreventedBy);
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -123,7 +123,8 @@
 return CFT_Device;
   } else if (hasAttr(D, IgnoreImplicitHDAttr)) {
 return CFT_Host;
-  } else if (D->isImplicit() && !IgnoreImplicitHDAttr) {
+  } else if ((D->isImplicit() || !D->isUserProvided()) &&
+ !IgnoreImplicitHDAttr) {
 // Some implicit declarations (like intrinsic functions) are not marked.
 // Set the most lenient target on them for maximal flexibility.
 return CFT_HostDevice;


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(t);
   test_hd(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/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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector PreventedBy;
   bool Result = Method->isUsualDeallocationFunction(PreventedBy);
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -123,7 +123,8 @@
 return CFT_Device;
   } else if (hasAttr(D, IgnoreImplicitHDAttr)) {
 return CFT_Host;
-  } else if (D->isImplicit

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

2021-01-20 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170
+  bool SkipDtorChecks = VD->getType()->isArrayType();
+
+  // CUDA: Skip destructor checks for host-only variables during device-side
+  // compilation
+  SkipDtorChecks |=
+  (LangOpts.CUDAIsDevice && VD->hasGlobalStorage() &&
+   !(VD->hasAttr() || VD->hasAttr() ||

rsmith wrote:
> tra wrote:
> > rsmith wrote:
> > > Is this safe? What happens if the destructor for the variable is a 
> > > template, and instantiating that template results in a reference to a 
> > > device function? Eg:
> > > 
> > > ```
> > > template __device__ void f() {}
> > > template struct A {
> > >   ~A() { f<<<>>>(); }
> > > };
> > > A a;
> > > ```
> > This is business as usual -- we catch it during host compilation, where `a` 
> > is instantiated.
> > 
> > ```
> > h.cu:3:10: error: no matching function for call to 'f'
> >   ~A() { f(); }
> >  ^~~~
> > h.cu:5:8: note: in instantiation of member function 'A::~A' requested 
> > here
> > A a;
> >^
> > h.cu:1:51: note: candidate function not viable: call to __device__ function 
> > from __host__ function
> > template __attribute__((device)) void f() {}
> > 
> > 1 error generated when compiling for host.
> > ```
> > 
> > If it were a `__device__ A a;` , then we catch it during GPU 
> > compilation and also complain that we can't have dynamic initializers.
> > 
> Sorry, testcase wasn't quite right; I meant for `f` to be `__global__` not 
> `__device__` so that the kernel call to it works. Fixed example:
> 
> ```
> extern "C" int cudaConfigureCall(int a, int b);
> template __attribute__((__global__)) void f(T) {}
> template struct A {
>   ~A() { f<<<1, 1>>>(T()); }
> };
> A a;
> ```
> 
> I think that this is valid. In order for it to work, we need to trigger 
> instantiation of `f` on the device side of the compilation. In order to 
> do that, we need to trigger instantiation of `A::~A()`, so we need to 
> mark it referenced on the device side. (This is, I think, in line with the 
> general principle that we want to do the same template instantiations of host 
> functions on both sides of the compilation, so that both sides agree on which 
> kernel functions are referenced.)
You're right.  To think of it this particular change is not needed at all any 
more. The real issue is fixed by the better selection of the usual deallocator. 
We do not need to skip dtor checks here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170
+  bool SkipDtorChecks = VD->getType()->isArrayType();
+
+  // CUDA: Skip destructor checks for host-only variables during device-side
+  // compilation
+  SkipDtorChecks |=
+  (LangOpts.CUDAIsDevice && VD->hasGlobalStorage() &&
+   !(VD->hasAttr() || VD->hasAttr() ||

tra wrote:
> rsmith wrote:
> > Is this safe? What happens if the destructor for the variable is a 
> > template, and instantiating that template results in a reference to a 
> > device function? Eg:
> > 
> > ```
> > template __device__ void f() {}
> > template struct A {
> >   ~A() { f<<<>>>(); }
> > };
> > A a;
> > ```
> This is business as usual -- we catch it during host compilation, where `a` 
> is instantiated.
> 
> ```
> h.cu:3:10: error: no matching function for call to 'f'
>   ~A() { f(); }
>  ^~~~
> h.cu:5:8: note: in instantiation of member function 'A::~A' requested 
> here
> A a;
>^
> h.cu:1:51: note: candidate function not viable: call to __device__ function 
> from __host__ function
> template __attribute__((device)) void f() {}
> 
> 1 error generated when compiling for host.
> ```
> 
> If it were a `__device__ A a;` , then we catch it during GPU compilation 
> and also complain that we can't have dynamic initializers.
> 
Sorry, testcase wasn't quite right; I meant for `f` to be `__global__` not 
`__device__` so that the kernel call to it works. Fixed example:

```
extern "C" int cudaConfigureCall(int a, int b);
template __attribute__((__global__)) void f(T) {}
template struct A {
  ~A() { f<<<1, 1>>>(T()); }
};
A a;
```

I think that this is valid. In order for it to work, we need to trigger 
instantiation of `f` on the device side of the compilation. In order to do 
that, we need to trigger instantiation of `A::~A()`, so we need to mark it 
referenced on the device side. (This is, I think, in line with the general 
principle that we want to do the same template instantiations of host functions 
on both sides of the compilation, so that both sides agree on which kernel 
functions are referenced.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-20 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170
+  bool SkipDtorChecks = VD->getType()->isArrayType();
+
+  // CUDA: Skip destructor checks for host-only variables during device-side
+  // compilation
+  SkipDtorChecks |=
+  (LangOpts.CUDAIsDevice && VD->hasGlobalStorage() &&
+   !(VD->hasAttr() || VD->hasAttr() ||

rsmith wrote:
> Is this safe? What happens if the destructor for the variable is a template, 
> and instantiating that template results in a reference to a device function? 
> Eg:
> 
> ```
> template __device__ void f() {}
> template struct A {
>   ~A() { f<<<>>>(); }
> };
> A a;
> ```
This is business as usual -- we catch it during host compilation, where `a` is 
instantiated.

```
h.cu:3:10: error: no matching function for call to 'f'
  ~A() { f(); }
 ^~~~
h.cu:5:8: note: in instantiation of member function 'A::~A' requested here
A a;
   ^
h.cu:1:51: note: candidate function not viable: call to __device__ function 
from __host__ function
template __attribute__((device)) void f() {}

1 error generated when compiling for host.
```

If it were a `__device__ A a;` , then we catch it during GPU compilation 
and also complain that we can't have dynamic initializers.




Comment at: clang/test/SemaCUDA/default-ctor.cu:28
   InHD inhd;
-  Out out; // expected-error{{no matching constructor for initialization of 
'Out'}}
+  Out out;
   OutD outd;

rsmith wrote:
> I don't think we should accept this -- only functions that are defaulted on 
> their first declaration should get special treatment. Instead of checking for 
> `isDefaulted()` above, you should check for `!isUserProvided()` instead.
Fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-20 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 317946.
tra added a comment.

Addressed Richard's comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  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(t);
   test_hd(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/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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector 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() || VD->hasAttr() ||
+ VD->hasAttr()));
+  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,8 @@
 return CFT_Device;
   } else if (hasAttr(D, IgnoreImplicitHDAttr)) {
 return CFT_Host;
-  } else if (D->isImplicit() && !IgnoreImplicitHDAttr) {
+  } else if ((D->isImplicit() || !D->isUserProvided()) &&
+ !IgnoreImplicitHDAttr) {
 // Some implicit declarations (like intrinsic functions) are not marked.
 // Set the most lenient target on them for maximal flexibility.
 return CFT_HostDevice;


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(t);
   test_hd(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/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(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

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

2021-01-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170
+  bool SkipDtorChecks = VD->getType()->isArrayType();
+
+  // CUDA: Skip destructor checks for host-only variables during device-side
+  // compilation
+  SkipDtorChecks |=
+  (LangOpts.CUDAIsDevice && VD->hasGlobalStorage() &&
+   !(VD->hasAttr() || VD->hasAttr() ||

Is this safe? What happens if the destructor for the variable is a template, 
and instantiating that template results in a reference to a device function? Eg:

```
template __device__ void f() {}
template struct A {
  ~A() { f<<<>>>(); }
};
A a;
```



Comment at: clang/test/SemaCUDA/default-ctor.cu:28
   InHD inhd;
-  Out out; // expected-error{{no matching constructor for initialization of 
'Out'}}
+  Out out;
   OutD outd;

I don't think we should accept this -- only functions that are defaulted on 
their first declaration should get special treatment. Instead of checking for 
`isDefaulted()` above, you should check for `!isUserProvided()` instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94732/new/

https://reviews.llvm.org/D94732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-01-14 Thread 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(t);
   test_hd(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(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(D)) {
+  if (IdentifyCUDAPreference(Caller, FD) > CFP_WrongSide)
+return false;
+}
+  }
+  // We've found no better variants.
+}
+  }
 
   SmallVector 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() || VD->hasAttr() ||
+ VD->hasAttr()));
+  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(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