myhsu added inline comments.
================ Comment at: clang/lib/Basic/Targets/M68k.cpp:77-79 + Builder.defineMacro("M68k"); + Builder.defineMacro("__M68k__"); + Builder.defineMacro("__M68K__"); ---------------- jrtc27 wrote: > myhsu wrote: > > jrtc27 wrote: > > > Where are these coming from? GCC only defines `__m68k__`. > > I think GCC does: > > https://github.com/gcc-mirror/gcc/blob/b90d051ecbc1d8972ae1bf0cd7588fcc66df0722/gcc/config/m68k/m68k.h#L64 > > > > Also I found this page: https://sourceforge.net/p/predef/wiki/Architectures > Yes, and as you can see none of those three are defined by GCC. Defining > `M68k` is particularly bad as it will collide with the C++ namespace used by > this very backend so you won't be able to build Clang with Clang on m68k! And > the others are just a waste of time. Defining fewer macros is best, that way > people don't come to rely on non-standard ones and instead use the single > option that exists everywhere (`__m68k__`). > it will collide with the C++ namespace used by this very backend so you won't > be able to build Clang with Clang on m68k good point :-) > Defining fewer macros is best, that way people don't come to rely on > non-standard ones and instead use the single option that exists everywhere fair enough CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88393/new/ https://reviews.llvm.org/D88393 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits