tra accepted this revision. tra added a comment. This revision is now accepted and ready to land.
In D124866#3501641 <https://reviews.llvm.org/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. ================ Comment at: clang/lib/Parse/ParseDecl.cpp:902 + while (Tok.is(tok::kw___noinline__)) { + IdentifierInfo *AttrName = Tok.getIdentifierInfo(); + SourceLocation AttrNameLoc = ConsumeToken(); ---------------- yaxunl wrote: > tra wrote: > > aaron.ballman wrote: > > > yaxunl wrote: > > > > aaron.ballman wrote: > > > > > I think we should we be issuing a pedantic "this is a clang > > > > > extension" warning here, WDYT? > > > > will do > > > I'm questioning whether my advice here was good or not -- now that I see > > > the CUDA spec already calls these function qualifiers... it's debatable > > > whether this is a Clang extension or just the way in which Clang > > > implements the CUDA function qualifiers. @tra -- do you have opinions? > > > > > > I'm sort of leaning towards dropping the extension warning, but the only > > > reason I can think of for keeping it is if Clang is the only CUDA > > > compiler that doesn't require you to include a header before using the > > > function qualifiers. If that's the case, there is a portability concern. > > I'm not sure if such a warning would be useful. > > > > > the only reason I can think of for keeping it is if Clang is the only > > > CUDA compiler that doesn't require you to include a header before using > > > the function qualifiers. If that's the case, there is a portability > > > concern. > > > > I don't think it's an issue. > > > > We already have similar divergence between nvcc/clang. E.g. built-in > > variables like `threadIdx`. Clang implements them in a header, but NVCC > > provides them by compiler itself. > > With both compilers the variables are available by the time we get to > > compile user code. Virtually all CUDA compilations are done with tons of > > CUDA headers pre-included by compiler. Those that do not do that are > > already on their own and have to provide many other 'standard' CUDA things > > like target attributes. I don't think we need to worry about that. > > > I can remove the diagnostics since it seems unnecessary. > > I tend to treat it as an extension since nvcc is the de facto standard > implementation, which does not implement it as a keyword. Compared to that, > this is like an extension. I'd argue that NVCC does implement it (as in "documents and makes it available"). Providing the documented functionality using a different implementation does not reach the point of being an extension, IMO. While there are observable differences between implementations, depending on them would be a portability error for the user. 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
