aaron.ballman added inline comments.
================ Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); - Builder.defineMacro("__ARM_FEATURE_CRC32", "1"); } ---------------- Hmm, is this correct? `__ARM_FEATURE_ATOMICS` is defined in one other place, but it's conditionally defined: https://github.com/llvm/llvm-project/blob/11926e6149d2a68ecb0652b248efe6890c163846/clang/lib/Basic/Targets/AArch64.cpp#L475 and `__ARM_FEATURE_CRC32` is defined in two places, both conditional: https://github.com/llvm/llvm-project/blob/11926e6149d2a68ecb0652b248efe6890c163846/clang/lib/Basic/Targets/AArch64.cpp#L422 and https://github.com/llvm/llvm-project/blob/11926e6149d2a68ecb0652b248efe6890c163846/clang/lib/Basic/Targets/ARM.cpp#L747 ================ Comment at: clang/lib/Basic/Targets/AArch64.cpp:343-347 - // Target properties. - if (!getTriple().isOSWindows() && getTriple().isArch64Bit()) { - Builder.defineMacro("_LP64"); - Builder.defineMacro("__LP64__"); - } ---------------- I think this is correct -- the other definition has a different predicate (https://github.com/llvm/llvm-project/blob/11926e6149d2a68ecb0652b248efe6890c163846/clang/lib/Frontend/InitPreprocessor.cpp#L949) but I think it ends up being define in exactly the same scenarios. ================ Comment at: clang/lib/Basic/Targets/VE.cpp:30-32 - Builder.defineMacro("unix", "1"); - Builder.defineMacro("__unix__", "1"); - Builder.defineMacro("__linux__", "1"); ---------------- Shouldn't this be calling `DefineStd(Builder, "unix", Opts);` like all the others? ================ Comment at: clang/lib/Basic/Targets/VE.cpp:35 Builder.defineMacro("__ve__", "1"); - Builder.defineMacro("__STDC_HOSTED__", "1"); - Builder.defineMacro("__STDC__", "1"); ---------------- This is technically a breaking change but I think it's a good breaking change. The VE target seems to imply that there's never a freestanding mode unlike the general utility: https://github.com/llvm/llvm-project/blob/11926e6149d2a68ecb0652b248efe6890c163846/clang/lib/Frontend/InitPreprocessor.cpp#L431 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150966/new/ https://reviews.llvm.org/D150966 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits