[PATCH] D88391: [M68k] (Patch 5/8) Target lowering

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


RKSimon added a comment.

Some more style cleanups



================
Comment at: llvm/lib/Target/M68k/M68kCollapseMOVEMPass.cpp:127
+    // Since MOVEM in control mode increment the address on each iteration
+    assert(Start != INT_MIN);
+    return Start;
----------------
Move the comment into the assert message?


================
Comment at: llvm/lib/Target/M68k/M68kCollapseMOVEMPass.cpp:161
+    auto End = State.end();
+    auto DL = MI->getDebugLoc();
+
----------------
 DebugLoc DL = 


================
Comment at: llvm/lib/Target/M68k/M68kExpandPseudo.cpp:275
+    } else {
+      assert(false && "Oh really? You need to pop that much?");
+      // RTD can only handle immediates as big as 2**16-1.  If we need to pop
----------------
A more useful assert message would be nice


================
Comment at: llvm/lib/Target/M68k/M68kISelDAGToDAG.cpp:529
+
+    if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(N0)) {
+      AM.GV = G->getGlobal();
----------------
(style) Consistently use auto* for cast/dyn_cast


================
Comment at: llvm/lib/Target/M68k/M68kInstrInfo.cpp:737
+      return load ? M68k::MOV16cp : M68k::MOV16pc;
+    }
+    llvm_unreachable("Unknown 1-byte regclass");
----------------
(style)
```
    if (M68k::DR8RegClass.hasSubClassEq(RC))
      return load ? M68k::MOVM8mp_P : M68k::MOVM8pm_P;
    if (M68k::CCRCRegClass.hasSubClassEq(RC))
      return load ? M68k::MOV16cp : M68k::MOV16pc;
```


================
Comment at: llvm/lib/Target/M68k/M68kInstrInfo.h:64
+  default:
+    llvm_unreachable("Illegal condition code!");
+  case M68k::COND_T:
----------------
(style) Remove default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/M68kInstrInfo.h:103
+  default:
+    llvm_unreachable("Illegal condition code!");
+  case M68k::COND_EQ:
----------------
(style) Remove default case and move llvm_unreachable after the switch statement


================
Comment at: llvm/lib/Target/M68k/M68kMCInstLower.cpp:119
+    LLVM_DEBUG(MI->dump());
+    llvm_unreachable("unknown operand type");
+  case MachineOperand::MO_Register:
----------------
(style) Move this after the switch statement


================
Comment at: llvm/lib/Target/M68k/M68kMCInstLower.cpp:165
+    default:
+      llvm_unreachable("Invalid opcode");
+    case M68k::TAILJMPj:
----------------
(style) Move this after the switch statement


================
Comment at: llvm/lib/Target/M68k/M68kSubtarget.cpp:186
+      return M68kII::MO_PC_RELATIVE_ADDRESS;
+    }
+  }
----------------
```
    if (isPositionIndependent())
      return M68kII::MO_GOTPCREL;
    return M68kII::MO_PC_RELATIVE_ADDRESS;
```


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

https://reviews.llvm.org/D88391



More information about the llvm-commits mailing list