[X86] Add peephole to convert `(Cmp Op32/Op64, Imm8)` -> `(Cmp Op16/Op8, Imm8)`

[goldstein.w.n]

Summary

This saves a bit of codesize.

This is valid if the sign-flags are not used:

• eq : https://alive2.llvm.org/ce/z/KUtebW
• ne : https://alive2.llvm.org/ce/z/fT0hBt
• ult: https://alive2.llvm.org/ce/z/s5FEYL
• ugt: https://alive2.llvm.org/ce/z/jaq6oS
• uge: https://alive2.llvm.org/ce/z/bYzJ3p
• ule: https://alive2.llvm.org/ce/z/8QXTiC

[goldstein.w.n]

Fix trunc

[RKSimon]

llvm/test/CodeGen/X86/vector-reduce-and-cmp.ll

691 ↗

(On Diff #516194)

This looks suspicous?

[goldstein.w.n]

llvm/test/CodeGen/X86/vector-reduce-and-cmp.ll

691 ↗

(On Diff #516194)

I think its from the C.trunc(NewVT.getScalarSizeInBits()).getZExtValue().

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5688

OpVT.isScalarInteger() should always bee true for X86ISD::CMP I think. The isa<ConstantSDNode>(N1) would guarantee it too.

5690

Do we need to check overflow flag uses too?

5694

aswell -> as well

5697

Space after if

5700

space after if

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5690

Maybe carry flag too?

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5709

Shouldn't the type for the constant be NewVT?

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5697

Shouldn't we check that OpVT.getScalarSizeInBits() is more than 8 bits before calling MaskedValueIsZero?

[goldstein.w.n]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5688

OpVT.isScalarInteger() should always bee true for X86ISD::CMP I think. The isa<ConstantSDNode>(N1) would guarantee it too.

There is CMPrr?

5690

I think yes for overflow flag, but I don't think we need for carry flag.

5709

Was doing that originally. Was running into:

MachineOperand.h:370: llvm::Register llvm::MachineOperand::getReg() const: Assertion `isReg() && "This is not a register operand!"' failed.

Phoebe suggested this.

[goldstein.w.n]

Fix some nits + also check for OF flag usage

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5688

I may have written that poorly. I meant that if isa<ConstantSDNode>(N1) was true, it implied N1 had scalar integer type. Thus making the type isScalarInteger redundant even if X86ISD::CMP didn't guarantee it.

5709

The types of the 2 arguments to X86ISD::CMP are supposed to be the same so there's some other issue going on.

llvm/test/CodeGen/X86/combine-movmsk.ll

44

Is this only a smaller encoding because it uses %al? For any other register I don't think it's smaller.

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5695

I think this can be isInt<8>(C.getSExtValue())

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5695

The alive proof is checking the constant is zero extended from 8 bits, right? But this check is a signed checked.

[goldstein.w.n]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5695

This is checking if C can be encoded as imm8.

llvm/test/CodeGen/X86/combine-movmsk.ll

44

Is this only a smaller encoding because it uses %al? For any other register I don't think it's smaller.

You're right:

	cmpl	$-1, %eax
    cmpw    $-1, %ax
    cmpb    $-1, %al

	cmpl	$-1, %ecx
    cmpw    $-1, %cx
    cmpb    $-1, %cl

Probably if anything we should do something to get rid of imm16 encoding. But think its fair to abandon this.

[pengfei]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5709

The problem was getConstant vs. getTargetConstant. I tried i8 vs. i32 before, but forgot to change it back. Seems the type here doesn't matter.

[craig.topper]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

5695

So it needs to be uint<8> to make the transform valid and an sint<8> to make the transform profitable. But we only checked the sint<8> condition?

[goldstein.w.n]

Craig pointed out, this only benefits for %eax -> %al. Given how niche it is, going to abandon.

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

MachineSDNode *emitPCMPISTR(unsigned ROpc, unsigned MOpc, bool MayFoldLoad,
                                 const SDLoc &dl, MVT VT, SDNode *Node);
     MachineSDNode *emitPCMPESTR(unsigned ROpc, unsigned MOpc, bool MayFoldLoad,
                                 const SDLoc &dl, MVT VT, SDNode *Node,
                                 SDValue &InGlue);
 
     bool tryOptimizeRem8Extend(SDNode *N);
 
     bool onlyUsesZeroFlag(SDValue Flags) const;
+
+    enum FlagClass { Sign, Overflow };
+
+    bool hasNoFlagClassUses(SDValue Flags, FlagClass FC) const;
+    bool hasNoOverflowFlagUses(SDValue Flags) const;
     bool hasNoSignFlagUses(SDValue Flags) const;
     bool hasNoCarryFlagUses(SDValue Flags) const;
   };
 }
 
 char X86DAGToDAGISel::ID = 0;
 
 INITIALIZE_PASS(X86DAGToDAGISel, DEBUG_TYPE, PASS_NAME, false, false)
for (SDNode::use_iterator FlagUI = UI->use_begin(),
       default:
         return false;
       }
     }
   }
   return true;
 }
 
-/// Test whether the given X86ISD::CMP node has any uses which require the SF
-/// flag to be accurate.
-bool X86DAGToDAGISel::hasNoSignFlagUses(SDValue Flags) const {
+static bool mayUseSignFlag(X86::CondCode CC) {
+  switch (CC) {
+    // Comparisons which don't examine the SF flag.
+    case X86::COND_A: case X86::COND_AE:
+    case X86::COND_B: case X86::COND_BE:
+    case X86::COND_E: case X86::COND_NE:
+    case X86::COND_O: case X86::COND_NO:
+    case X86::COND_P: case X86::COND_NP:
+      return false;
+    // Anything else: assume conservatively.
+    default:
+      return true;
+    }
+}
+
+static bool mayUseOverflowFlag(X86::CondCode CC) {
+  switch (CC) {
+    // Comparisons which don't examine the SF flag.
+    case X86::COND_A: case X86::COND_AE:
+    case X86::COND_B: case X86::COND_BE:
+    case X86::COND_E: case X86::COND_NE:
+    case X86::COND_P: case X86::COND_NP:
+    case X86::COND_S: case X86::COND_NS:
+    case X86::COND_L: case X86::COND_GE:
+    case X86::COND_G: case X86::COND_LE:
+      return false;
+    // Anything else: assume conservatively.
+    default:
+      return true;
+    }
+}
+enum FlagClass {
+    Sign,
+    Overflow
+};
+
+/// Test whether the given X86ISD::CMP node has any uses which require a flag
+/// from FlagClass
+bool X86DAGToDAGISel::hasNoFlagClassUses(SDValue Flags, FlagClass FC) const {
     // Examine each user of the node.
     for (SDNode::use_iterator UI = Flags->use_begin(), UE = Flags->use_end();
          UI != UE; ++UI) {
       // Only check things that use the flags.
       if (UI.getUse().getResNo() != Flags.getResNo())
       continue;
       // Only examine CopyToReg uses that copy to EFLAGS.
       if (UI->getOpcode() != ISD::CopyToReg ||
           cast<RegisterSDNode>(UI->getOperand(1))->getReg() != X86::EFLAGS)
       return false;
       // Examine each user of the CopyToReg use.
       for (SDNode::use_iterator FlagUI = UI->use_begin(),
-           FlagUE = UI->use_end(); FlagUI != FlagUE; ++FlagUI) {
+                                FlagUE = UI->use_end();
+           FlagUI != FlagUE; ++FlagUI) {
       // Only examine the Flag result.
-      if (FlagUI.getUse().getResNo() != 1) continue;
+      if (FlagUI.getUse().getResNo() != 1)
+        continue;
       // Anything unusual: assume conservatively.
-      if (!FlagUI->isMachineOpcode()) return false;
+      if (!FlagUI->isMachineOpcode())
+        return false;
       // Examine the condition code of the user.
       X86::CondCode CC = getCondFromNode(*FlagUI);
 
-      switch (CC) {
-      // Comparisons which don't examine the SF flag.
-      case X86::COND_A: case X86::COND_AE:
-      case X86::COND_B: case X86::COND_BE:
-      case X86::COND_E: case X86::COND_NE:
-      case X86::COND_O: case X86::COND_NO:
-      case X86::COND_P: case X86::COND_NP:
-        continue;
-      // Anything else: assume conservatively.
-      default:
+      if (FC == FlagClass::Sign && mayUseSignFlag(CC))
+        return false;
+      else if (FC == FlagClass::Overflow && mayUseOverflowFlag(CC))
         return false;
-      }
       }
     }
     return true;
 }
 
+bool X86DAGToDAGISel::hasNoSignFlagUses(SDValue Flags) const {
+    return hasNoFlagClassUses(Flags, FlagClass::Sign);
+}
+
+bool X86DAGToDAGISel::hasNoOverflowFlagUses(SDValue Flags) const {
+    return hasNoFlagClassUses(Flags, FlagClass::Overflow);
+}
+
 static bool mayUseCarryFlag(X86::CondCode CC) {
   switch (CC) {
   // Comparisons which don't examine the CF flag.
   case X86::COND_O: case X86::COND_NO:
   case X86::COND_E: case X86::COND_NE:
   case X86::COND_S: case X86::COND_NS:
   case X86::COND_P: case X86::COND_NP:
   case X86::COND_L: case X86::COND_GE:
case X86ISD::STRICT_FCMPS: {
     CurDAG->RemoveDeadNode(Node);
     return;
   }
 
   case X86ISD::CMP: {
     SDValue N0 = Node->getOperand(0);
     SDValue N1 = Node->getOperand(1);
 
+    EVT OpVT = N0.getValueType();
+    // This is a fixup if we converted (cmp Op8/Op16, i8) -> (cmp Op32, i32)
+    // during lowering.
+    if (OpVT.getScalarSizeInBits() > 8 && isa<ConstantSDNode>(N1) &&
+        !isNullConstant(N1) && hasNoSignFlagUses(SDValue(Node, 0)) &&
+        hasNoOverflowFlagUses(SDValue(Node, 0))) {
+      const APInt &C = cast<ConstantSDNode>(N1)->getAPIntValue();
+      // Only do replacement if the constant can get imm8 encoding. Imm16 values
+      // cause LCP stalls in the frontend.
+      // TODO: Enable imm16 transform as well if -Os is set?
+      if (C.getSignificantBits() <= 8) {
+        EVT NewVT = OpVT;
+        if (CurDAG->MaskedValueIsZero(
+                N0, APInt::getBitsSetFrom(OpVT.getScalarSizeInBits(), 8)))
+          NewVT = MVT::i8;
+        else if (CurDAG->MaskedValueIsZero(
+                     N0, APInt::getBitsSetFrom(OpVT.getScalarSizeInBits(), 16)))
+          NewVT = MVT::i16;
+
+        if (NewVT == OpVT)
+          break;
+
+        SDValue TruncN0 = CurDAG->getZExtOrTrunc(N0, dl, NewVT);
+        insertDAGNode(*CurDAG, SDValue(Node, 0), TruncN0);
+        SDValue TruncN1 = CurDAG->getConstant(
+            C.trunc(NewVT.getScalarSizeInBits()).getZExtValue(), dl, MVT::i32);
+        insertDAGNode(*CurDAG, SDValue(Node, 0), TruncN1);
+        SDValue NewCmp =
+            CurDAG->getNode(X86ISD::CMP, dl, MVT::i32, TruncN0, TruncN1);
+        ReplaceNode(Node, NewCmp.getNode());
+        if (N1.getNode()->use_empty())
+          CurDAG->RemoveDeadNode(N1.getNode());
+        SelectCode(NewCmp.getNode());
+        return;
+      }
+    }
+
     // Optimizations for TEST compares.
     if (!isNullConstant(N1))
       break;
 
     // Save the original VT of the compare.
     MVT CmpVT = N0.getSimpleValueType();
 
     // If we are comparing (and (shr X, C, Mask) with 0, emit a BEXTR followed

llvm/test/CodeGen/X86/combine-movmsk.ll

 }
 
 define i1 @movmskps_allof_bitcast_v2f64(<2 x double> %a0) {
 ; SSE-LABEL: movmskps_allof_bitcast_v2f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    xorpd %xmm1, %xmm1
 ; SSE-NEXT:    cmpeqpd %xmm0, %xmm1
 ; SSE-NEXT:    movmskpd %xmm1, %eax
-; SSE-NEXT:    cmpl $3, %eax
+; SSE-NEXT:    cmpb $3, %al
 ; SSE-NEXT:    sete %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: movmskps_allof_bitcast_v2f64:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:    vcmpeqpd %xmm0, %xmm1, %xmm0
 ; AVX-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
; AVX-NEXT:    retq
   ret i1 %5
 }
 
 define i1 @pmovmskb_allof_bitcast_v2i64(<2 x i64> %a0) {
 ; SSE2-LABEL: pmovmskb_allof_bitcast_v2i64:
 ; SSE2:       # %bb.0:
 ; SSE2-NEXT:    pshufd {{.*#+}} xmm0 = xmm0[1,1,3,3]
 ; SSE2-NEXT:    movmskps %xmm0, %eax
-; SSE2-NEXT:    cmpl $15, %eax
+; SSE2-NEXT:    cmpb $15, %al
 ; SSE2-NEXT:    sete %al
 ; SSE2-NEXT:    retq
 ;
 ; SSE42-LABEL: pmovmskb_allof_bitcast_v2i64:
 ; SSE42:       # %bb.0:
 ; SSE42-NEXT:    movmskpd %xmm0, %eax
-; SSE42-NEXT:    cmpl $3, %eax
+; SSE42-NEXT:    cmpb $3, %al
 ; SSE42-NEXT:    sete %al
 ; SSE42-NEXT:    retq
 ;
 ; AVX-LABEL: pmovmskb_allof_bitcast_v2i64:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:    vtestpd %xmm1, %xmm0
 ; AVX-NEXT:    setb %al

 }
 
 define i1 @pmovmskb_allof_bitcast_v4f32(<4 x float> %a0) {
 ; SSE-LABEL: pmovmskb_allof_bitcast_v4f32:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    xorps %xmm1, %xmm1
 ; SSE-NEXT:    cmpeqps %xmm0, %xmm1
 ; SSE-NEXT:    movmskps %xmm1, %eax
-; SSE-NEXT:    cmpl $15, %eax
+; SSE-NEXT:    cmpb $15, %al
 ; SSE-NEXT:    sete %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: pmovmskb_allof_bitcast_v4f32:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vxorps %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:    vcmpeqps %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1