[X86] combineCarryThroughADD - add support for X86::COND_A commutations (PR24545)

[RKSimon]

Summary

As discussed on PR24545, we should try to commute X86::COND_A 'icmp ugt' cases to X86::COND_B 'icmp ult' to more optimally bind the carry flag output to a SBB instruction.

[craig.topper]

LGTM

[craig.topper]

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

34904

Do we need to make sure the data output of the subtract doesn't have any users? CarryOp1.hasOneUse() will only check the flag result has one use. CarryOp1->hasOneUse() will check that the entire node only has one use.

[RKSimon]

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

34904

I can do that - interestingly the code in combineAddOrSubToADCOrSBB (where I got this from) doesn't do this either

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

static bool checkBoolTestAndOrSetCCCombine(SDValue Cond, X86::CondCode &CC0,
   CC1 = (X86::CondCode)SetCC1->getConstantOperandVal(0);
   Flags = SetCC0->getOperand(1);
   return true;
 }
 
 // When legalizing carry, we create carries via add X, -1
 // If that comes from an actual carry, via setcc, we use the
 // carry directly.
-static SDValue combineCarryThroughADD(SDValue EFLAGS) {
+static SDValue combineCarryThroughADD(SDValue EFLAGS, SelectionDAG &DAG) {
   if (EFLAGS.getOpcode() == X86ISD::ADD) {
     if (isAllOnesConstant(EFLAGS.getOperand(1))) {
       SDValue Carry = EFLAGS.getOperand(0);
       while (Carry.getOpcode() == ISD::TRUNCATE ||
              Carry.getOpcode() == ISD::ZERO_EXTEND ||
              Carry.getOpcode() == ISD::SIGN_EXTEND ||
              Carry.getOpcode() == ISD::ANY_EXTEND ||
              (Carry.getOpcode() == ISD::AND &&
               isOneConstant(Carry.getOperand(1))))
         Carry = Carry.getOperand(0);
       if (Carry.getOpcode() == X86ISD::SETCC ||
           Carry.getOpcode() == X86ISD::SETCC_CARRY) {
-        if (Carry.getConstantOperandVal(0) == X86::COND_B)
-          return Carry.getOperand(1);
+        // TODO: Merge this code with equivalent in combineAddOrSubToADCOrSBB?
+        uint64_t CarryCC = Carry.getConstantOperandVal(0);
+        SDValue CarryOp1 = Carry.getOperand(1);
+        if (CarryCC == X86::COND_B)
+          return CarryOp1;
+        if (CarryCC == X86::COND_A) {
+          // Try to convert COND_A into COND_B in an attempt to facilitate
+          // materializing "setb reg".
+          //
+          // Do not flip "e > c", where "c" is a constant, because Cmp
+          // instruction cannot take an immediate as its first operand.
+          //
+          if (CarryOp1.getOpcode() == X86ISD::SUB && CarryOp1.hasOneUse() &&
+              CarryOp1.getValueType().isInteger() &&
+              !isa<ConstantSDNode>(CarryOp1.getOperand(1))) {
+            SDValue SubCommute =
+                DAG.getNode(X86ISD::SUB, SDLoc(CarryOp1), CarryOp1->getVTList(),
+                            CarryOp1.getOperand(1), CarryOp1.getOperand(0));
+            return SDValue(SubCommute.getNode(), CarryOp1.getResNo());
+          }
+        }
       }
     }
   }
 
   return SDValue();
 }
 
 /// Optimize an EFLAGS definition used according to the condition code \p CC
 /// into a simpler EFLAGS value, potentially returning a new \p CC and replacing
 /// uses of chain values.
 static SDValue combineSetCCEFLAGS(SDValue EFLAGS, X86::CondCode &CC,
                                   SelectionDAG &DAG,
                                   const X86Subtarget &Subtarget) {
   if (CC == X86::COND_B)
-    if (SDValue Flags = combineCarryThroughADD(EFLAGS))
+    if (SDValue Flags = combineCarryThroughADD(EFLAGS, DAG))
       return Flags;
 
   if (SDValue R = checkBoolTestSetCCCombine(EFLAGS, CC))
     return R;
   return combineSetCCAtomicArith(EFLAGS, CC, DAG, Subtarget);
 }
 
 /// Optimize X86ISD::CMOV [LHS, RHS, CONDCODE (e.g. X86::COND_NE), CONDVAL]
static SDValue combineX86AddSub(SDNode *N, SelectionDAG &DAG) {
   SDValue LHS = N->getOperand(0);
   SDValue RHS = N->getOperand(1);
   SDValue Res = DAG.getNode(X86ISD::ADD == N->getOpcode() ? ISD::ADD : ISD::SUB,
                             DL, LHS.getSimpleValueType(), LHS, RHS);
   return DAG.getMergeValues({Res, DAG.getConstant(0, DL, MVT::i32)}, DL);
 }
 
 static SDValue combineSBB(SDNode *N, SelectionDAG &DAG) {
-  if (SDValue Flags = combineCarryThroughADD(N->getOperand(2))) {
+  if (SDValue Flags = combineCarryThroughADD(N->getOperand(2), DAG)) {
     MVT VT = N->getSimpleValueType(0);
     SDVTList VTs = DAG.getVTList(VT, MVT::i32);
     return DAG.getNode(X86ISD::SBB, SDLoc(N), VTs,
                        N->getOperand(0), N->getOperand(1),
                        Flags);
   }
 
   // Fold SBB(SUB(X,Y),0,Carry) -> SBB(X,Y,Carry)
SDValue Res1 = DAG.getNode(ISD::AND, DL, VT,
                                DAG.getNode(X86ISD::SETCC_CARRY, DL, VT,
                                            DAG.getConstant(X86::COND_B, DL,
                                                            MVT::i8),
                                            N->getOperand(2)),
                                DAG.getConstant(1, DL, VT));
     return DCI.CombineTo(N, Res1, CarryOut);
   }
 
-  if (SDValue Flags = combineCarryThroughADD(N->getOperand(2))) {
+  if (SDValue Flags = combineCarryThroughADD(N->getOperand(2), DAG)) {
     MVT VT = N->getSimpleValueType(0);
     SDVTList VTs = DAG.getVTList(VT, MVT::i32);
     return DAG.getNode(X86ISD::ADC, SDLoc(N), VTs,
                        N->getOperand(0), N->getOperand(1),
                        Flags);
   }
 
   return SDValue();

llvm/trunk/test/CodeGen/X86/combine-sbb.ll

 declare  { i64, i1 } @llvm.usub.with.overflow.i64(i64, i64)
 
 ; PR24545 less_than_ideal()
 define i8 @PR24545(i32, i32, i32* nocapture readonly) {
 ; X86-LABEL: PR24545:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    movl (%ecx), %edx
-; X86-NEXT:    cmpl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    seta %dl
-; X86-NEXT:    addb $-1, %dl
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
+; X86-NEXT:    cmpl (%ecx), %edx
 ; X86-NEXT:    sbbl 4(%ecx), %eax
 ; X86-NEXT:    setb %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: PR24545:
 ; X64:       # %bb.0:
-; X64-NEXT:    cmpl %edi, (%rdx)
-; X64-NEXT:    seta %al
-; X64-NEXT:    addb $-1, %al
+; X64-NEXT:    cmpl (%rdx), %edi
 ; X64-NEXT:    sbbl 4(%rdx), %esi
 ; X64-NEXT:    setb %al
 ; X64-NEXT:    retq
   %4 = load i32, i32* %2
   %5 = icmp ugt i32 %4, %0
   %6 = zext i1 %5 to i8
   %7 = getelementptr inbounds i32, i32* %2, i32 1
   %8 = load i32, i32* %7
   %9 = tail call { i8, i32 } @llvm.x86.subborrow.32(i8 %6, i32 %1, i32 %8)
   %10 = extractvalue { i8, i32 } %9, 0
   %11 = icmp ne i8 %10, 0
   %12 = zext i1 %11 to i8
   ret i8 %12
 }
 declare { i8, i32 } @llvm.x86.subborrow.32(i8, i32, i32)