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

Mariya Podchishchaeva via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 16 05:34:57 PDT 2019


Fznamznon added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:286
+help.
+  }];
+}
----------------
Anastasia wrote:
> 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.
Generally the `sycl_device` attribute will be added automatically by the compiler. But as @bader mentioned before:
> we might need to use sycl_device attribute to mark functions, which are called from the different translation units, i.e. compiler can't identify it w/o user's help.
> SYCL specification proposes to use special macro as "device function marker", but I guess we can have additional "spellings" in the clang.
I think It would be better to re-use this attribute in implementation of "device function marker" macro from SYCL spec than implement additional logic in the compiler to handle this macro. So I saved possibility to add this attribute in code.






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