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

2022-05-11 Thread Dan Liew via Phabricator via cfe-commits
delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, yaxunl wrote: > yaxunl wrote: > > delcypher wrote: > > > yaxunl wro

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

2022-05-11 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, yaxunl wrote: > delcypher wro

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

2022-05-11 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, delcypher wrote: > yaxunl wrote: > > delcypher wrote: > > > @yaxunl Is

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

2022-05-10 Thread Dan Liew via Phabricator via cfe-commits
delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, yaxunl wrote: > delcypher wrote: > > @yaxunl Is it intentional that

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

2022-05-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, delcypher wrote: > @yaxunl Is it intentional that you didn't update `K

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

2022-05-10 Thread Dan Liew via Phabricator via cfe-commits
delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, @yaxunl Is it intentional that you didn't update `KEYALL` here? Tha

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

2022-05-10 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. yaxunl marked an inline comment as done. Closed by commit rGafc9d674fe5a: [CUDA][HIP] support __noinline__ as keyword (authored by yaxunl). Herald added a project: clan

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

2022-05-10 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments. Comment at: clang/test/SemaCUDA/noinline.cu:8 +__attribute__((noinline)) void fun2() { } +__attribute__((__noinline__)) void fun3() { } aaron.ballman wrote: > yaxunl wrote: > > aaron.ballman wrote: > > > yaxunl wrote: > > > > aaron.ball

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

2022-05-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 428423. yaxunl marked an inline comment as done. yaxunl added a comment. make it a feature, add tests for pedantic, fix release notes and doecumentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 Files

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

2022-05-10 Thread 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: > > > >

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

2022-05-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 4 inline comments as done. yaxunl added inline comments. Comment at: clang/test/SemaCUDA/noinline.cu:8 +__attribute__((noinline)) void fun2() { } +__attribute__((__noinline__)) void fun3() { } aaron.ballman wrote: > yaxunl wrote: > > aaron.ballman w

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

2022-05-10 Thread 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: > > > >

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

2022-05-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 4 inline comments as done. yaxunl added inline comments. Comment at: clang/docs/ReleaseNotes.rst:344-345 -CUDA Language Changes in Clang +CUDA/HIP Language Changes in Clang -- aaron.ballman wrote: > will fix =

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

2022-05-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D124866#3501641 , @yaxunl wrote: > If we are to add `__forceinline__` as a keyword, I feel it better be a > separate patch to be cleaner. I'm fine with that. A few nits and a question about the test recently added.

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

2022-05-09 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. In D124866#3501641 , @yaxunl wrote: > If we are to add `__forceinline__` as a keyword, I feel it better be a > separate patch to be cleaner. Fine with me.

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

2022-05-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 428167. yaxunl marked 2 inline comments as done. yaxunl added a comment. removed diagnostics and added more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 Files: clang/docs/ReleaseNotes.rst clang

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

2022-05-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 4 inline comments as done. yaxunl added a comment. In D124866#3501203 , @aaron.ballman wrote: >> `__forceinline__` does not have the issue as `__noinline__` has since it is >> not a GCC attribute. The current CUDA/HIP implementation of >>

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

2022-05-09 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In 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 `__force

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

2022-05-09 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/test/CodeGenCUDA/noinline.cu:1 +// optimization is needed, otherwise by default all functions have noinline. + aaron.ballman wrote: > I've asked @erichkeane to weigh in on whether there's a better approach here

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

2022-05-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a subscriber: erichkeane. aaron.ballman added a comment. In D124866#3501181 , @yaxunl wrote: > In D124866#3500761 , @aaron.ballman > wrote: > >> Should we do `__forceinline__` at the same time

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

2022-05-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D124866#3500761 , @aaron.ballman wrote: > Should we do `__forceinline__` at the same time so that there's consistency? `__forceinline__` does not have the issue as `__noinline__` has since it is not a GCC attribute. The curre

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

2022-05-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D124866#3497439 , @tra wrote: >> CUDA/HIP do not have language spec. > > Well. It's not completely true. CUDA programming guide does serve as the > de-facto spec for CUDA. It's far from perfect, but it does mention > `_

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

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D124866#3497439 , @tra wrote: >> CUDA/HIP do not have language spec. > > Well. It's not completely true. CUDA programming guide does serve as the > de-facto spec for CUDA. It's far from perfect, but it does mention > `__noinli

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

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. > CUDA/HIP do not have language spec. Well. It's not completely true. CUDA programming guide does serve as the de-facto spec for CUDA. It's far from perfect, but it does mention `__noinline__` and `__forceinline__` as function qualifiers: https://docs.nvidia.com/cuda/cuda-

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

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 427700. yaxunl marked an inline comment as done. yaxunl added a comment. added release note and documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 Files: clang/docs/ReleaseNotes.rst clang/inc

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

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/include/clang/Basic/Features.def:274 +// CUDA/HIP Features +FEATURE(cuda_noinline_keyword, true) + aaron.ballman wrote: > yaxunl wrote: > > aaron.ballman wrote: > > > Do the

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

2022-05-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Features.def:274 +// CUDA/HIP Features +FEATURE(cuda_noinline_keyword, true) + yaxunl wrote: > aaron.ballman wrote: > > Do the CUDA or HIP specs define `__noinline__` as a keyword specific

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

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 427685. yaxunl marked 4 inline comments as done. yaxunl added a comment. revised by Aaron's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 Files: clang/include/clang/Basic/Attr.td clang/includ

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

2022-05-06 Thread 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<"noinl

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

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a reviewer: rsmith. tra added a subscriber: rsmith. tra 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? Summoning @rsmith for his lang

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

2022-05-06 Thread Aaron Ballman via Phabricator via cfe-commits
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, a

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

2022-05-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 426794. yaxunl added a comment. add feature cuda_noinline_keyword to facilitate CUDA/HIP headers removing __noinline__ macro CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 Files: clang/include/clang/Bas

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

2022-05-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision. yaxunl added a reviewer: tra. Herald added a reviewer: aaron.ballman. Herald added subscribers: mattd, carlosgalvezp, dexonsmith. Herald added a project: All. yaxunl requested review of this revision. CUDA/HIP programs use `__noinline__` like a keyword e.g. `__noinlin