[PATCH] D88390: [M68k] (Patch 4/8) MC layer and object file support

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 24 13:06:59 PST 2021


RKSimon added a comment.

A few minors, but overall I think this is good enough to go in.

@rengolin @jrtc27 Any more comments?



================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kELFObjectWriter.cpp:51
+  default:
+    llvm_unreachable("Unimplemented");
+  case FK_Data_4:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kELFObjectWriter.cpp:78
+    default:
+      llvm_unreachable("Unrecognized size");
+    case RT_32:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kELFObjectWriter.cpp:89
+    default:
+      llvm_unreachable("Unrecognized size");
+    case RT_32:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kELFObjectWriter.cpp:101
+    default:
+      llvm_unreachable("Unrecognized size");
+    case RT_32:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kELFObjectWriter.cpp:112
+    default:
+      llvm_unreachable("Unrecognized size");
+    case RT_32:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kFixupKinds.h:23
+  default:
+    llvm_unreachable("invalid fixup kind!");
+  case FK_PCRel_1:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kFixupKinds.h:42
+  default:
+    llvm_unreachable("Invalid generic fixup size!");
+  case 8:
----------------
(style) Drop the default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kMCCodeEmitter.cpp:117
+                                      const MCSubtargetInfo &STI) const {
+  bool DA, Reg;
+  switch (Bead & 0xF) {
----------------
Can you get uninitialized variables further down? The switch statement isn't exhaustive so static analyzers will complain, add a default llvm_unreachable?


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kMCCodeEmitter.cpp:212
+    Addendum -= 2;
+  }
+
----------------
drop braces


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

https://reviews.llvm.org/D88390



More information about the llvm-commits mailing list