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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kInstPrinter.cpp:84
+  unsigned HalfMask, Reg;
+  for (int s = 0; s < 8; s += 8) {
+    HalfMask = Mask >> s;
----------------
`for (int s = 0; s < 8; s += 8)` ?


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kInstPrinter.cpp:86
+    HalfMask = Mask >> s;
+    if (HalfMask && s != 0) {
+      O << ',';
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Applies to many braces in this file.


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kInstPrinter.cpp:181
+
+void M68kInstPrinter::printPCDMem(const MCInst *MI, uint64_t Address,
+                                    int opNum, raw_ostream &O) {
----------------
The PCREL_OPERAND style `print*` functions are usually used this way: D77853


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kMCCodeEmitter.cpp:58
+
+  unsigned EncodeBits(unsigned ThisByte, uint8_t Bead, const MCInst &MI,
+                      const MCInstrDesc &Desc, uint64_t &Buffer,
----------------
`encodeBits`

Newer backends should stick with the function naming coding standard


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kMCTargetDesc.cpp:70
+  }
+  return createM68kMCSubtargetInfoImpl(TT, CPU, /*TuneCPU*/ CPU, ArchFS);
+}
----------------
clang-format recognized spelling is `/*TuneCPU=*/CPU`


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kMCTargetDesc.cpp:83
+  // Initial state of the frame pointer is SP+stackGrowth.
+  MCCFIInstruction Inst = MCCFIInstruction::cfiDefCfa(
+      nullptr, MRI.getDwarfRegNum(llvm::M68k::SP, true), -stackGrowth);
----------------
There should be some `.cfi_*` tests with this change.


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

https://reviews.llvm.org/D88390