[X86] Simplify cmp-zext-constant DAG patterns.

[bryant]

Summary

This cuts down on the spurious generation of MOV32rr and should be correct for the E, NE, P, NP condition codes. However, I am unsure how useful this might be since movl uses zero micro-ops.

I've added nadav since s/he appears in grep -i x86 CODE_OWNERS.txt.

[bryant]

Added test case.

[bryant]

Revert accident deletion

[RKSimon]

test/CodeGen/X86/cmp-zext-combine.ll

21

Please can you simplify the test to focus just on the combine in question. And preferably add a wider range of tests touching different comparisons / integer sizes (inc ones that shouldn't combine).

If possible use utils/update_llc_test_checks.py to generate full code output (you will need to add check prefixes to the 32/64 tests)

[spatel]

lib/Target/X86/X86ISelLowering.cpp

30277–30282

Usually (always?) you would start at the outer op/instruction/node of a pattern that you want to match. Is there some reason not to implement this starting in combineSetCC() rather than combineX86Cmp()?

[spatel]

Just noticed that llvm-commits is not subscribed to this review. It's probably best to abandon this review and start again with llvm-commits subscribed from the outset.

[bryant]

lib/Target/X86/X86ISelLowering.cpp

30277–30282

• in order for this transformation to occur, there must be an ISD::ZERO_EXTEND present in the inner node.

• often, however, the zext doesn't appear until DAGCombiner generates it somewhere down the line in the combine process -- after the corresponding parent setcc has been touched by combineSetCC/combineX86SetCC;
  • ISD::ZERO_EXTEND is set to Expand in this file, although it won't matter much since there are many other patterns that need DAGCombiner to distill into a zext.

• when DAGCombiner combines, it re-adds to the work list only the direct users of the zext node. this would be the cmp but not the setcc.

[bryant]

Upon closer inspection, I've discovered that the true root of the problem lies in DAGCombiner failing to recognize and combine (srl (shl, c), c). So I will be re-submitting the fix there instead.

[RKSimon]

Abandon this given D22726?

[bryant]

yes.

lib/Target/X86/X86ISelLowering.cpp

                                  EFLAGS));
 }
 
+// transforms (setcc CC, (cmp (zext:N x), const)) to (setcc CC (cmp x, const'))
+// where N is some bit width greater than x's; const' has x's bit width; const
+// == const' in value
+static SDValue combineX86Cmp(SDNode *N, SelectionDAG &DAG,
+                             TargetLowering::DAGCombinerInfo &DCI,
+                             const X86Subtarget &Subtarget) {
+  if (N->uses().begin() != N->uses().end() &&
+      all_of(N->uses().begin(), N->uses().end(),
+             [](const SDNode *user) {
+               X86::CondCode cc = X86::CondCode(user->getConstantOperandVal(0));
+               return user->getOpcode() == X86ISD::SETCC &&
+                      (cc == X86::COND_E || cc == X86::COND_NE ||
+                       cc == X86::COND_NE || cc == X86::COND_NP ||
+                       cc == X86::COND_P);
+             }) &&
+      N->getOperand(0).getOpcode() == ISD::ZERO_EXTEND) {
+    if (ConstantSDNode *rhs = dyn_cast<ConstantSDNode>(N->getOperand(1))) {
+      const SDValue &innerlhs = N->getOperand(0)->getOperand(0);
+      uint64_t rhsval = rhs->getZExtValue();
+      if (isUIntN(innerlhs.getValueSizeInBits(), rhsval)) {
+        return DAG.getNode(
+            X86ISD::CMP, SDLoc(N), innerlhs.getValueType(), innerlhs,
+            DAG.getConstant(rhsval, SDLoc(rhs), innerlhs.getValueType()));
+      }
+    }
+  }
+  return SDValue();
+}
+
 // Optimize  RES = X86ISD::SETCC CONDCODE, EFLAG_INPUT
 static SDValue combineX86SetCC(SDNode *N, SelectionDAG &DAG,
                                TargetLowering::DAGCombinerInfo &DCI,

   case ISD::SIGN_EXTEND:    return combineSext(N, DAG, DCI, Subtarget);
   case ISD::SIGN_EXTEND_INREG: return combineSignExtendInReg(N, DAG, Subtarget);
   case ISD::SETCC:          return combineSetCC(N, DAG, Subtarget);
+  case X86ISD::CMP:         return combineX86Cmp(N, DAG, DCI, Subtarget);
   case X86ISD::SETCC:       return combineX86SetCC(N, DAG, DCI, Subtarget);
   case X86ISD::BRCOND:      return combineBrCond(N, DAG, DCI, Subtarget);
   case X86ISD::VZEXT:       return combineVZext(N, DAG, DCI, Subtarget);

test/CodeGen/X86/cmp-zext-combine.ll

+; RUN: llc -mtriple=x86_64 < %s | FileCheck %s
+; RUN: llc -mtriple=i686 < %s | FileCheck %s
+
+define i32 @f(i64*, i32, i32) {
+; CHECK-LABEL: f
+  %4 = alloca i64*, align 8
+  %5 = bitcast i64** %4 to i8*
+  store i64* %0, i64** %4, align 8
+  %6 = call { i32, i32 } asm sideeffect "cmpxchg8b $2", "={ax},={dx},=*m,{bx},{cx},0,1,*m,~{flags},~{dirflag},~{fpsr},~{flags}"(i64** nonnull %4, i32 4, i32 5, i32 0, i32 0, i64** nonnull %4) #5
+  %7 = extractvalue { i32, i32 } %6, 0
+  %8 = extractvalue { i32, i32 } %6, 1
+  %9 = zext i32 %8 to i64
+  %10 = shl nuw i64 %9, 32
+; CHECK-DAG: testl %eax, %eax
+  %11 = icmp ne i32 %7, 0
+; CHECK-DAG: testl %edx, %edx
+  %12 = icmp ugt i64 %10, 4294967295
+  %13 = and i1 %11, %12
+  %14 = zext i1 %13 to i32
+  ret i32 %14
+}