aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM with some testing requests.



================
Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:4-5
+
+__attribute((sycl_kernel)) void foo() {
+}
----------------
aaron.ballman wrote:
> Missing some tests:
> * test that both attributes can be applied to whatever subjects they 
> appertain to
> * test that neither attribute can be applied to an incorrect subject
> * test that the attributes do not accept arguments
> * test that the attribute is ignored when SYCL is not enabled
> 
> Are there situations where the attribute does not make sense, such as member 
> functions, virtual functions, etc? If so, those are good test cases (and 
> diagnostics) to add as well.
Still missing a test that the attribute is ignored when SYCL is not enabled.


================
Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:6
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning 
{{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' 
attribute only applies to function templates}}
----------------
This test should be on a templated function (we already demonstrated it only 
applies to templated functions, so the check for the argument is not what is 
failing).


================
Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:7
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning 
{{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' 
attribute only applies to function templates}}
+
----------------
Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

Reply via email to