[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.

[Aaron Ballman via Phabricator via cfe-commits]

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:
> > 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.
> > Then why are we introducing __declspec(__device__) rather than a keyword attribute __device__?
> 
> `__device__` is a macro defined in the CUDA headers, which must include and we are not able to modify.
Okay, it makes sense as to why we can't have a keyword spelling, but it also doesn't answer why we need the `__declspec` spelling for it. Are you saying that there are CUDA headers out there where this attribute is spelled with `__attribute__` and others with `__declspec`? If so, then this change makes a bit more sense to me.


================
Comment at: clang/include/clang/Basic/Attr.td:610
   let LangOpts = [CUDA];
   let Documentation = [Undocumented];
 }
----------------
jlebar wrote:
> aaron.ballman wrote:
> > 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?
> > how is a user to know what Clang does or does not support?
> 
> These macros are fundamental to CUDA support.  The statement "you can compile CUDA code with clang" will immediately imply to every CUDA developer in existence the statement "you can use `__device__` in your code".
Yet we add new CUDA attributes periodically, and CUDA comes out with new versions and new features.

Looking at the CUDA docs, I see `__managed__`, but I don't see such an attribute in Clang. How is a user to know whether we do/don't support such a construct?


https://reviews.llvm.org/D28321