[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D124866#3501203 <https://reviews.llvm.org/D124866#3501203>, @aaron.ballman wrote:

>>> Should we do `__forceinline__` at the same time so that there's consistency?
>
> Primarily to reduce user confusion. It's kind of weird for `__noinline__` to be a keyword and `__forceinline__` to not be a keyword when they're both defined the same way by the CUDA spec. This means you can #undef one of them but not the other, that sort of thing.

I'm slightly biased towards making them both a keyword. That said, I may be convinced otherwise if we discover that it may break some assumptions in existing C++ code. I just don't know enough.



================
Comment at: clang/lib/Parse/ParseDecl.cpp:902
+  while (Tok.is(tok::kw___noinline__)) {
+    IdentifierInfo *AttrName = Tok.getIdentifierInfo();
+    SourceLocation AttrNameLoc = ConsumeToken();
----------------
aaron.ballman wrote:
> yaxunl wrote:
> > aaron.ballman wrote:
> > > I think we should we be issuing a pedantic "this is a clang extension" warning here, WDYT?
> > will do
> I'm questioning whether my advice here was good or not -- now that I see the CUDA spec already calls these function qualifiers... it's debatable whether this is a Clang extension or just the way in which Clang implements the CUDA function qualifiers. @tra -- do you have opinions?
> 
> I'm sort of leaning towards dropping the extension warning, but the only reason I can think of for keeping it is if Clang is the only CUDA compiler that doesn't require you to include a header before using the function qualifiers. If that's the case, there is a portability concern.
I'm not sure if such a warning would be useful. 

> the only reason I can think of for keeping it is if Clang is the only CUDA compiler that doesn't require you to include a header before using the function qualifiers. If that's the case, there is a portability concern.

I don't think it's an issue.

We already have similar divergence between nvcc/clang. E.g. built-in variables like `threadIdx`. Clang implements them in a header, but NVCC provides them by compiler itself. 
With both compilers the variables are available by the time we get to compile user code. Virtually all CUDA compilations are done with tons of CUDA headers pre-included by compiler. Those that do not do that are already on their own and have to provide many other 'standard' CUDA things like target attributes. I don't think we need to worry about that.



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

https://reviews.llvm.org/D124866