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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:1778-1779
 def NoInline : DeclOrStmtAttr {
-  let Spellings = [GCC<"noinline">, CXX11<"clang", "noinline">,
+  let Spellings = [Keyword<"__noinline__">, GCC<"noinline">, CXX11<"clang", "noinline">,
                    C2x<"clang", "noinline">, Declspec<"noinline">];
   let Accessors = [Accessor<"isClangNoInline", [CXX11<"clang", "noinline">,
----------------
aaron.ballman wrote:
> 
will do


================
Comment at: clang/include/clang/Basic/Features.def:274
+// CUDA/HIP Features
+FEATURE(cuda_noinline_keyword, true)
+
----------------
aaron.ballman wrote:
> Do the CUDA or HIP specs define `__noinline__` as a keyword specifically? If not, this isn't a `FEATURE`, it's an `EXTENSION` because it's specific to Clang, not the language standard.
CUDA/HIP do not have language spec. In their programming guide, they do not define `__noinline__` as a keyword.

Will make it an extension.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:902
+  while (Tok.is(tok::kw___noinline__)) {
+    IdentifierInfo *AttrName = Tok.getIdentifierInfo();
+    SourceLocation AttrNameLoc = ConsumeToken();
----------------
aaron.ballman wrote:
> I think we should we be issuing a pedantic "this is a clang extension" warning here, WDYT?
will do


================
Comment at: clang/test/SemaCUDA/noinline.cu:8
+__attribute__((noinline)) void fun2() { }
+__attribute__((__noinline__)) void fun3() { }
----------------
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


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

https://reviews.llvm.org/D124866