[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Looking forward to see m68k support (and hopefully sega genesis toolchain support someday)!



================
Comment at: clang/lib/Basic/Targets/M68k.cpp:73
+void M68kTargetInfo::getTargetDefines(const LangOptions &Opts,
+                                        MacroBuilder &Builder) const {
+  using llvm::Twine;
----------------
This doesn't seem to match the coding style, `clang-format` to the rescue.


================
Comment at: clang/lib/Basic/Targets/M68k.cpp:89
+  case CK_68010:
+    RawDef = "mc68010";
+    break;
----------------
Can you make all the `defineMacro` inline within the `case`s? Clarity seems more useful here.


================
Comment at: clang/lib/Basic/Targets/M68k.h:35
+    CK_68040
+    // CK_68060
+  } CPU = CK_Unknown;
----------------
Can you remove the comment and add the enum back whenever you introduce `CK_68060` support?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88393/new/

https://reviews.llvm.org/D88393