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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 6 05:59:32 PDT 2022


aaron.ballman added a comment.

I don't know how language extensions come about in CUDA or HIP -- is there an appropriate standards body (or something similar) that's aware of this extension and supports it?

The changes should likely come with a release note entry about the new functionality, and some documentation changes as well.



================
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">,
----------------



================
Comment at: clang/include/clang/Basic/Features.def:274
+// CUDA/HIP Features
+FEATURE(cuda_noinline_keyword, true)
+
----------------
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.


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


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


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

https://reviews.llvm.org/D124866



More information about the cfe-commits mailing list