[PATCH] D60455: [SYCL] Add support for SYCL device attributes

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 05:43:56 PDT 2019


aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

These attributes are being parsed and silently ignored -- are there semantics expected for the attributes?



================
Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;
----------------
Is there a reason to not also introduce a C++11 and C2x style spelling in the `clang` namespace? e.g., `[[clang::sycl_device]]`


================
Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
----------------
No new, undocumented attributes, please.


================
Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:4-5
+
+__attribute((sycl_kernel)) void foo() {
+}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455





More information about the cfe-commits mailing list