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

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu 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;
----------------
MaskRay wrote:
> `for (int s = 0; s < 8; s += 8)` ?
good catch, thanks


================
Comment at: llvm/lib/Target/M68k/MCTargetDesc/M68kInstPrinter.cpp:181
+
+void M68kInstPrinter::printPCDMem(const MCInst *MI, uint64_t Address,
+                                    int opNum, raw_ostream &O) {
----------------
MaskRay wrote:
> The PCREL_OPERAND style `print*` functions are usually used this way: D77853
Well...actually Motorola is using its own assembly language (and where this `(N,%pc)` syntax comes from). We're trying to conform with that in order to have better coordination with other existing toolchains


================
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);
----------------
MaskRay wrote:
> There should be some `.cfi_*` tests with this change.
Correct, it's on the TODO list now


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

https://reviews.llvm.org/D88390