yaxunl added inline comments.
================
Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:318
Builder.defineMacro("__AMDGCN_WAVEFRONT_SIZE", Twine(WavefrontSize));
+ Builder.defineMacro("__AMDGCN_CUMODE", Twine(CUMode));
}
----------------
arsenm wrote:
> Why do we sometimes use __ on both sides, and sometimes only leading __?
We did not establish a naming convention for pre-defined macros.
The rationale of only prefixing with `__` is that:
1. it is shorter
2. many other targets following that for target feature related macros, e.g.
https://github.com/llvm/llvm-project/blob/main/clang/test/Preprocessor/arm-target-features.c
,
https://github.com/llvm/llvm-project/blob/main/clang/test/Preprocessor/riscv-target-features.c
The rationale of adding `__` on both sides is:
1. most gcc predefined macros follow this convention
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
2. some targets follow this convention for target feature related macros, e.g
https://github.com/llvm/llvm-project/blob/main/clang/test/Preprocessor/x86_target_features.c
I think it is not too late to establish a naming convention for predefined
macros for amdgpu target. I would prefer to adding `__` on both sides since
1. most of the existing amdgpu predefined macros follow that. We just need to
add `__AMDGCN_WAVEFRONT_SIZE__` and phasing out `__AMDGCN_WAVEFRONT_SIZE`. This
will be the least interruptive.
2. we will conform to GCC naming convention for pre-defined macros.
Or maybe we should have an RFC for this topic.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145343/new/
https://reviews.llvm.org/D145343
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
