sameerds added a comment. Can you please submit a squashed single commit for review against the master branch? All the recent commits seem to be relative to previous commits, and I am having trouble looking at the change as a whole.
The commit description needs some fixes: 1. Remove use of Title Case, in places like "using Fence instruction" and "LLVM Atomic Memory Ordering" and "as a String". They are simply common nouns, not proper nouns. When in doubt, look at the LangRef to see how these words are used as common nouns. 2. Don't mention that enum values are okay too. I am sure they will always be okay, but it's better to encourage the use of symbols. 3. Don't mention HSA even if the AMDGPU user manual does so. 4. In fact the last two sentences in the description are not strictly necessary ... they are the logical outcome of the scopes being target-specific strings. 5. Note that the general practice in documentation is to say "AMDGPU" which covers the LLVM Target, while "amdgcn" is only used in the code because it is one of multiple architectures in the AMDGPU backend. ================ Comment at: clang/docs/LanguageExtensions.rst:2458 +AMDGCN specific builtins +------------------------- ---------------- We probably don't need to document the new builtin here. Clearly, we have not documented any other AMDGPU builtin, and there is no need to start now. If necessary, we can take that up as a separate task covering all builtins. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75917/new/ https://reviews.llvm.org/D75917 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
