aaron.ballman added a subscriber: erichkeane. aaron.ballman added a comment.
In D124866#3501181 <https://reviews.llvm.org/D124866#3501181>, @yaxunl wrote: > In D124866#3500761 <https://reviews.llvm.org/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 current CUDA/HIP implementation of `__forceinline__` > in header files is sufficient. I do not see the benefit of implementing > `__forceinline__` as a keyword. 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. ================ Comment at: clang/test/CodeGenCUDA/noinline.cu:1 +// optimization is needed, otherwise by default all functions have noinline. + ---------------- I've asked @erichkeane to weigh in on whether there's a better approach here than specifying an optimization level. ================ Comment at: clang/test/SemaCUDA/noinline.cu:8 +__attribute__((noinline)) void fun2() { } +__attribute__((__noinline__)) void fun3() { } ---------------- 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() {} ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124866/new/ https://reviews.llvm.org/D124866 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
