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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/test/SemaCUDA/noinline.cu:8
+__attribute__((noinline)) void fun2() { }
+__attribute__((__noinline__)) void fun3() { }
----------------
yaxunl wrote:
> aaron.ballman wrote:
> > yaxunl wrote:
> > > aaron.ballman wrote:
> > > > yaxunl wrote:
> > > > > aaron.ballman wrote:
> > > > > > I think there should also be a test like:
> > > > > > ```
> > > > > > [[gnu::__noinline__]] void fun4() {}
> > > > > > ```
> > > > > > to verify that the double square bracket syntax also correctly handles this being a keyword now (I expect the test to pass).
> > > > > will do
> > > > Ah, I just noticed we also have no tests for the behavior of the keyword in the presence of the macro being defined. e.g.,
> > > > ```
> > > > #define __noinline__ __attribute__((__noinline__))
> > > > __noinline__ void fun5() {}
> > > > ```
> > > will do
> > I missed an important detail -- I think this is now going to generate a warning in `-pedantic` mode (through `-Wkeyword-macro`) when compiling for CUDA; is that going to be a problem for CUDA headers, or are those always included as a system header (and so the diagnostics will be suppressed)?
> I could not find how clang driver adds CUDA include path
> 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Cuda.cpp#L284
> 
> @tra do you know how CUDA include path is added? is it done by CMake? 
> 
> For HIP the HIP include path is added as a system include path by clang driver.
Whatever we find out, we can emulate its behavior here in the test file to see what the diagnostic behavior will be (you can use GNU linemarkers to convince the compiler parts of the source are in a system header).


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

https://reviews.llvm.org/D124866