[M68k] Fix CMP pattern matching and emission logic

CMP nodes with an immediate operand always have the immediate on the
right-hand side. But the pattern defined in TableGen had the immediate
on the left-hand side, causing the match to fail, so this has been
corrected.

Also, `emitCmp()` contained logic ported from X86 that is not relevant
to M68k, such as imposing restrictions on which ValueTypes can be
compared, so that logic has been removed. It would also replace CMP with
SUB to enable a specific optimization, but (as of now) on M68k it
usually results in a register copy before the SUB, which loses
efficiency.

With these changes, CMP instructions more reliably take advantage of
their addressing modes, and redundancy is significantly reduced.

Author: dansalvato

llvm/lib/Target/M68k/M68kISelLowering.cpp

@@ -1910,8 +1910,8 @@ SDValue M68kTargetLowering::EmitTest(SDValue Op, unsigned M68kCC,
   // we prove that the arithmetic won't overflow, we can't use OF or CF.
   if (Op.getResNo() != 0 || NeedOF || NeedCF) {
     // Emit a CMP with 0, which is the TEST pattern.
-    return DAG.getNode(M68kISD::CMP, DL, MVT::i8,
-                       DAG.getConstant(0, DL, Op.getValueType()), Op);
+    return DAG.getNode(M68kISD::CMP, DL, MVT::i8, Op,
+                       DAG.getConstant(0, DL, Op.getValueType()));
   }
   unsigned Opcode = 0;
   unsigned NumOperands = 0;
@@ -2068,8 +2068,8 @@ SDValue M68kTargetLowering::EmitTest(SDValue Op, unsigned M68kCC,
 
   if (Opcode == 0) {
     // Emit a CMP with 0, which is the TEST pattern.
-    return DAG.getNode(M68kISD::CMP, DL, MVT::i8,
-                       DAG.getConstant(0, DL, Op.getValueType()), Op);
+    return DAG.getNode(M68kISD::CMP, DL, MVT::i8, Op,
+                       DAG.getConstant(0, DL, Op.getValueType()));
   }
   SDVTList VTs = DAG.getVTList(Op.getValueType(), MVT::i8);
   SmallVector<SDValue, 4> Ops(Op->op_begin(), Op->op_begin() + NumOperands);
@@ -2107,24 +2107,7 @@ SDValue M68kTargetLowering::EmitCmp(SDValue Op0, SDValue Op1, unsigned M68kCC,
   assert(!(isa<ConstantSDNode>(Op1) && Op0.getValueType() == MVT::i1) &&
          "Unexpected comparison operation for MVT::i1 operands");
 
-  if ((Op0.getValueType() == MVT::i8 || Op0.getValueType() == MVT::i16 ||
-       Op0.getValueType() == MVT::i32 || Op0.getValueType() == MVT::i64)) {
-    // Only promote the compare up to I32 if it is a 16 bit operation
-    // with an immediate.  16 bit immediates are to be avoided.
-    if ((Op0.getValueType() == MVT::i16 &&
-         (isa<ConstantSDNode>(Op0) || isa<ConstantSDNode>(Op1))) &&
-        !DAG.getMachineFunction().getFunction().hasMinSize()) {
-      unsigned ExtendOp =
-          isM68kCCUnsigned(M68kCC) ? ISD::ZERO_EXTEND : ISD::SIGN_EXTEND;
-      Op0 = DAG.getNode(ExtendOp, DL, MVT::i32, Op0);
-      Op1 = DAG.getNode(ExtendOp, DL, MVT::i32, Op1);
-    }
-    // Use SUB instead of CMP to enable CSE between SUB and CMP.
-    SDVTList VTs = DAG.getVTList(Op0.getValueType(), MVT::i8);
-    SDValue Sub = DAG.getNode(M68kISD::SUB, DL, VTs, Op0, Op1);
-    return SDValue(Sub.getNode(), 1);
-  }
-  return DAG.getNode(M68kISD::CMP, DL, MVT::i8, Op0, Op1);
+  return DAG.getNode(M68kISD::CMP, DL, Op0.getValueType(), Op0, Op1);
 }
 
 /// Result of 'and' or 'trunc to i1' is compared against zero.
@@ -2305,7 +2288,7 @@ SDValue M68kTargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
       }
 
       Cmp = DAG.getNode(M68kISD::CMP, DL, MVT::i8,
-                        DAG.getConstant(1, DL, CmpOp0.getValueType()), CmpOp0);
+                        CmpOp0, DAG.getConstant(1, DL, CmpOp0.getValueType()));
 
       SDValue Res = // Res = 0 or -1.
           DAG.getNode(M68kISD::SETCC_CARRY, DL, Op.getValueType(),

llvm/lib/Target/M68k/M68kInstrArithmetic.td

@@ -394,7 +394,7 @@ defm XOR : MxBiArOp_DF_EAd<"eor", MxXor, 0xB, 0xA>;
 // CMP
 //===----------------------------------------------------------------------===//
 
-let Defs = [CCR] in {
+let isCompare = 1, Defs = [CCR] in {
 class MxCmp_RR<MxType LHS_TYPE, MxType RHS_TYPE = LHS_TYPE>
     : MxInst<(outs), (ins LHS_TYPE.ROp:$lhs, RHS_TYPE.ROp:$rhs),
              "cmp."#RHS_TYPE.Prefix#"\t$lhs, $rhs",
@@ -414,7 +414,7 @@ class MxCmp_RR<MxType LHS_TYPE, MxType RHS_TYPE = LHS_TYPE>
 class MxCmp_RI<MxType TYPE>
     : MxInst<(outs), (ins TYPE.IOp:$imm, TYPE.ROp:$reg),
              "cmpi."#TYPE.Prefix#"\t$imm, $reg",
-             [(set CCR, (MxCmp TYPE.IPat:$imm, TYPE.VT:$reg))]> {
+             [(set CCR, (MxCmp TYPE.VT:$reg, TYPE.IPat:$imm))]> {
   let Inst = (ascend
     (descend 0b00001100,
              !cast<MxEncSize>("MxEncSize"#TYPE.Size).Value,
@@ -433,7 +433,7 @@ class MxCmp_MI<MxType TYPE, MxOperand MEMOpd, ComplexPattern MEMPat,
                MxEncMemOp MEM_ENC>
     : MxInst<(outs), (ins TYPE.IOp:$imm, MEMOpd:$mem),
              "cmpi."#TYPE.Prefix#"\t$imm, $mem",
-             [(set CCR, (MxCmp TYPE.IPat:$imm, (load MEMPat:$mem)))]> {
+             [(set CCR, (MxCmp (load MEMPat:$mem), TYPE.IPat:$imm))]> {
   let Inst = (ascend
     (descend 0b00001100,
              !cast<MxEncSize>("MxEncSize"#TYPE.Size).Value,
@@ -449,8 +449,8 @@ class MxCmp_MI<MxType TYPE, MxOperand MEMOpd, ComplexPattern MEMPat,
 class MxCmp_BI<MxType TYPE>
     : MxInst<(outs), (ins TYPE.IOp:$imm, MxAL32:$abs),
              "cmpi."#TYPE.Prefix#"\t$imm, $abs",
-             [(set CCR, (MxCmp TYPE.IPat:$imm,
-                               (load (i32 (MxWrapper tglobaladdr:$abs)))))]> {
+             [(set CCR, (MxCmp (load (i32 (MxWrapper tglobaladdr:$abs))),
+                                TYPE.IPat:$imm))]> {
   defvar AbsEncoding = MxEncAddrMode_abs<"abs", true>;
   let Inst = (ascend
     (descend 0b00001100,