[PATCH] D83893: [CUDA][HIP] Always defer diagnostics for wrong-sided reference

2020-07-17 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4fc752b30b9a: [CUDA][HIP] Always defer diagnostics for 
wrong-sided reference (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83893

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/test/SemaCUDA/builtins.cu
  clang/test/SemaCUDA/call-kernel-from-kernel.cu
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/function-target.cu
  clang/test/SemaCUDA/implicit-device-lambda.cu
  clang/test/SemaCUDA/method-target.cu
  clang/test/SemaCUDA/reference-to-kernel-fn.cu

Index: clang/test/SemaCUDA/reference-to-kernel-fn.cu
===
--- clang/test/SemaCUDA/reference-to-kernel-fn.cu
+++ clang/test/SemaCUDA/reference-to-kernel-fn.cu
@@ -1,12 +1,14 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify \
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=host \
+// RUN:   -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify=dev \
 // RUN:   -verify-ignore-unexpected=note %s
-// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify \
-// RUN:   -verify-ignore-unexpected=note -DDEVICE %s
 
 // Check that we can reference (get a function pointer to) a __global__
 // function from the host side, but not the device side.  (We don't yet support
 // device-side kernel launches.)
 
+// host-no-diagnostics
+
 #include "Inputs/cuda.h"
 
 struct Dummy {};
@@ -17,13 +19,11 @@
 
 __host__ __device__ fn_ptr_t get_ptr_hd() {
   return kernel;
-#ifdef DEVICE
-  // expected-error@-2 {{reference to __global__ function}}
-#endif
+  // dev-error@-1 {{reference to __global__ function}}
 }
 __host__ fn_ptr_t get_ptr_h() {
   return kernel;
 }
 __device__ fn_ptr_t get_ptr_d() {
-  return kernel;  // expected-error {{reference to __global__ function}}
+  return kernel;  // dev-error {{reference to __global__ function}}
 }
Index: clang/test/SemaCUDA/method-target.cu
===
--- clang/test/SemaCUDA/method-target.cu
+++ clang/test/SemaCUDA/method-target.cu
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify=host,expected %s
+// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify=dev,expected %s
 
 #include "Inputs/cuda.h"
 
@@ -6,11 +7,11 @@
 // Test 1: host method called from device function
 
 struct S1 {
-  void method() {} // expected-note {{'method' declared here}}
+  void method() {} // dev-note {{'method' declared here}}
 };
 
 __device__ void foo1(S1& s) {
-  s.method(); // expected-error {{reference to __host__ function 'method' in __device__ function}}
+  s.method(); // dev-error {{reference to __host__ function 'method' in __device__ function}}
 }
 
 //--
@@ -29,22 +30,22 @@
 // Test 3: device method called from host function
 
 struct S3 {
-  __device__ void method() {} // expected-note {{'method' declared here}}
+  __device__ void method() {} // host-note {{'method' declared here}}
 };
 
 void foo3(S3& s) {
-  s.method(); // expected-error {{reference to __device__ function 'method' in __host__ function}}
+  s.method(); // host-error {{reference to __device__ function 'method' in __host__ function}}
 }
 
 //--
 // Test 4: device method called from host function
 
 struct S4 {
-  __device__ void method() {}  // expected-note {{'method' declared here}}
+  __device__ void method() {}  // host-note {{'method' declared here}}
 };
 
 __host__ __device__ void foo4(S4& s) {
-  s.method(); // expected-error {{reference to __device__ function 'method' in __host__ __device__ function}}
+  s.method(); // host-error {{reference to __device__ function 'method' in __host__ __device__ function}}
 }
 
 //--
@@ -63,9 +64,9 @@
 // Test 6: call method through pointer
 
 struct S6 {
-  void method() {} // expected-note {{'method' declared here}};
+  void method() {} // dev-note {{'method' declared here}};
 };
 
 __device__ void foo6(S6* s) {
-  s->method(); // expected-error {{reference to __host__ function 'method' in __device__ function}}
+  s->method(); // dev-error {{reference to __host__ function 'method' in __device__ function}}
 }
Index: clang/test/SemaCUDA/implicit-device-lambda.cu
===
--- clang/test/SemaCUDA/implicit-device-lambda.cu
+++ clang/test/SemaCUDA/implicit-device-lambda.cu
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -verify -fsyntax-only -verify-ignore-unexpected=warning -verify-ignore-unexpected=note %s
-// RUN: %clang_cc1 -std=c++11 -verify -fsyntax-only 

[PATCH] D83893: [CUDA][HIP] Always defer diagnostics for wrong-sided reference

2020-07-16 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D83893



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


[PATCH] D83893: [CUDA][HIP] Always defer diagnostics for wrong-sided reference

2020-07-15 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment.

tra and I talked offline and I...think this makes sense.


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

https://reviews.llvm.org/D83893



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


[PATCH] D83893: [CUDA][HIP] Always defer diagnostics for wrong-sided reference

2020-07-15 Thread Artem Belevich via Phabricator via cfe-commits
tra added subscribers: jlebar, rsmith.
tra added a comment.

> This is different from nvcc behavior, where it is diagnosed only if the 
> function is really emitted:

That by itself is not a sufficient reason for relaxing the checks. Clang is 
stricter/more principled in diagnosing other questionalbe things nvcc lets 
through, even when it should not have. 
NVCC only sees part of the source, so it's limited in what it can diagnose 
during particular compilation side. Clang can do better.

> Current clang behavior causes diagnostics for wrong-sided reference emitted 
> twice, once in host compilation, once in device compilation, which is 
> unnecessary and cluttering the screen.

I don't think it's a problem. Clang will abort compilation if one of 
sub-compilation fails. We'll only see the error once.

> More importantly, current clang behavior causes false alarms for valid use 
> cases:

I'm not convinced that your example is valid. To me it looks like `test` should 
not have been a HD, given that it always uses a host function.

That said, I can see where it may be useful in case of `test` being a constexpr 
template function (for example somewhere in a standard library) parametrized by 
something else which may be host or device-only.
E.g. this may be a somewhat better example: https://godbolt.org/z/44vPv8

OK. I think it is useful, but I could also use a second opinion. 
@rsmith , @jlebar do you have any thoughts on this?


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

https://reviews.llvm.org/D83893



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


[PATCH] D83893: [CUDA][HIP] Always defer diagnostics for wrong-sided reference

2020-07-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: kristof.beyls.

When a device function calls a host function or vice versa, this is wrong-sided
reference. Currently clang immediately diagnose it. This is different from nvcc
behavior, where it is diagnosed only if the function is really emitted:

  void foo();
  inline __device__ void bar() { foo(); }

https://godbolt.org/z/4G8P31

To me nvcc behavior makes more sense, since we do not need a diagnostic
unless we really need it. Current clang behavior causes diagnostics for 
wrong-sided
reference emitted twice, once in host compilation, once in device compilation,
which is unnecessary and cluttering the screen.

More importantly, current clang behavior causes false alarms for valid use 
cases:

  __device__ void bar(int x);
  
  template
  __device__ void foo(T x) {
bar(x);
  }
  
  struct A {
int a;
operator int() { return a; }
  };
  
  void foo(A a) {}
  
  template
  __device__ __host__ void test(T t) {
foo(t);
  }
  
  int main() {
A a;
test(a);
return 0;
  } 

In this example, we only emit `test` on host, it should compile.
However since `test` is a host device function, it triggers
instantiation of `foo` in device compilation. `foo` calls
a host `operator int()` which causes immediate diagnostics.

In this case, user may never intend to use `foo` on device side.
The instantiation of `foo` is a side effect of instantiation of
host device function `test`, therefore any diagnostics incurred
by `foo` should be deferred.

This patch let clang always defer diagnostics for wrong-sided
reference.


https://reviews.llvm.org/D83893

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/test/SemaCUDA/builtins.cu
  clang/test/SemaCUDA/call-kernel-from-kernel.cu
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/function-target.cu
  clang/test/SemaCUDA/implicit-device-lambda.cu
  clang/test/SemaCUDA/method-target.cu
  clang/test/SemaCUDA/reference-to-kernel-fn.cu

Index: clang/test/SemaCUDA/reference-to-kernel-fn.cu
===
--- clang/test/SemaCUDA/reference-to-kernel-fn.cu
+++ clang/test/SemaCUDA/reference-to-kernel-fn.cu
@@ -1,12 +1,14 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify \
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=host \
+// RUN:   -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify=dev \
 // RUN:   -verify-ignore-unexpected=note %s
-// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify \
-// RUN:   -verify-ignore-unexpected=note -DDEVICE %s
 
 // Check that we can reference (get a function pointer to) a __global__
 // function from the host side, but not the device side.  (We don't yet support
 // device-side kernel launches.)
 
+// host-no-diagnostics
+
 #include "Inputs/cuda.h"
 
 struct Dummy {};
@@ -17,13 +19,11 @@
 
 __host__ __device__ fn_ptr_t get_ptr_hd() {
   return kernel;
-#ifdef DEVICE
-  // expected-error@-2 {{reference to __global__ function}}
-#endif
+  // dev-error@-1 {{reference to __global__ function}}
 }
 __host__ fn_ptr_t get_ptr_h() {
   return kernel;
 }
 __device__ fn_ptr_t get_ptr_d() {
-  return kernel;  // expected-error {{reference to __global__ function}}
+  return kernel;  // dev-error {{reference to __global__ function}}
 }
Index: clang/test/SemaCUDA/method-target.cu
===
--- clang/test/SemaCUDA/method-target.cu
+++ clang/test/SemaCUDA/method-target.cu
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify=host,expected %s
+// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify=dev,expected %s
 
 #include "Inputs/cuda.h"
 
@@ -6,11 +7,11 @@
 // Test 1: host method called from device function
 
 struct S1 {
-  void method() {} // expected-note {{'method' declared here}}
+  void method() {} // dev-note {{'method' declared here}}
 };
 
 __device__ void foo1(S1& s) {
-  s.method(); // expected-error {{reference to __host__ function 'method' in __device__ function}}
+  s.method(); // dev-error {{reference to __host__ function 'method' in __device__ function}}
 }
 
 //--
@@ -29,22 +30,22 @@
 // Test 3: device method called from host function
 
 struct S3 {
-  __device__ void method() {} // expected-note {{'method' declared here}}
+  __device__ void method() {} // host-note {{'method' declared here}}
 };
 
 void foo3(S3& s) {
-  s.method(); // expected-error {{reference to __device__ function 'method' in __host__ function}}
+  s.method(); // host-error {{reference to __device__ function 'method' in __host__ function}}
 }
 
 //--
 // Test 4: device method called from host function
 
 struct S4 {
-  __device__ void method() {}  // expected-note