aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:604
 
+// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__).
+
----------------
jlebar wrote:
> aaron.ballman wrote:
> > For my own edification, do you have a link to some documentation of what 
> > CUDA attributes are spelled with `__declspec`?
> I do not believe such documentation exists.  It is an implementation detail 
> in the CUDA headers -- users never write `__attribute__((device))` or 
> `__declspec(__device__)`.  Instead, they write `__device__`.
Then why are we introducing `__declspec(__device__)` rather than a keyword 
attribute `__device__`?

My biggest concern is: I would like to verify that these actually should be 
supported as a `__declspec` attribute. From my simple testing in MSVC, it does 
not appear to support `__declspec(__device__)`, but perhaps I am doing it wrong 
(I'm mostly unfamiliar with CUDA). If this isn't something MSVC supports, then 
it is the first attribute we're supporting with a __declspec spelling that is 
not actually a Microsoft attribute, which is something worth discussing.


================
Comment at: clang/include/clang/Basic/Attr.td:610
   let LangOpts = [CUDA];
   let Documentation = [Undocumented];
 }
----------------
jlebar wrote:
> aaron.ballman wrote:
> > Now would be a good time to add the documentation for these attributes, 
> > otherwise there's a lot less chance users will know what ways they can 
> > spell the attributes (or what the attribute do).
> See above, these are an implementation detail.
We can still document that we support the attributes under their macro names, 
or do users not typically think of these macros as being attributes? I am 
mostly concerned about discoverability of the attributes -- how is a user to 
know what Clang does or does not support?


https://reviews.llvm.org/D28321



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

Reply via email to