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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:286
+help.
+  }];
+}
----------------
keryell wrote:
> aaron.ballman wrote:
> > I'm still not entirely certain how I would know what to mark and how. From the description, it sounds like whoever authors `parallel_for` needs to do this marking, or it somehow happens automatically?
> > 
> > (I'll do another editorial pass once I understand the intended behavior a bit better -- I expect there will be a few more wording issues to address.)
> In normal SYCL it happens automatically.
> In the compiler unit-tests it is done manually to exercise the framework.
> I am the one who suggested that in some other contexts, it could be used manually for some special purpose like using some weird hardware, but I do not want to derail the main review with this.
> In normal SYCL it happens automatically.
> In the compiler unit-tests it is done manually to exercise the framework.
> 


I think if they are not to be exposed to the user they should have no spelling. There are plenty of other ways to test this. For example AST dump.


> I am the one who suggested that in some other contexts, it could be used manually for some special purpose like using some weird hardware, but I do not want to derail the main review with this.

If you are suggesting to expose this feature then you are starting some sort of a language extensions and its use should be documented in some way. I am not sure about this but I think we will end up with some sort of a language extension for SYCL anyways because as it stands now it's not aligned with the general concept of C/C++ language design. So perhaps it's not entirely unreasonable to expose this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455