[X86] Fix wrong target specific combine on SETCC nodes.
authorAndrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Thu, 12 Mar 2015 15:16:58 +0000 (15:16 +0000)
committerAndrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Thu, 12 Mar 2015 15:16:58 +0000 (15:16 +0000)
Part of the folding logic implemented by function 'PerformISDSETCCCombine'
only worked under the assumption that the condition code in input could have
been either SETNE or SETEQ.
Unfortunately that assumption was incorrect, and in some cases the algorithm
ended up incorrectly folding SETCC nodes.

The incorrect folding only affected SETCC dag nodes where:
 - one of the operands was a build_vector of all zeroes;
 - the other operand was a SIGN_EXTEND from a vector of MVT:i1 elements;
 - the condition code was neither SETNE nor SETEQ.

Example:
  (setcc (v4i32 (sign_extend v4i1:%A)), (v4i32 VectorOfAllZeroes), setge)

Before this patch, the entire dag node sequence from the example was
incorrectly folded to node %A.

With this patch, the dag node sequence is folded to a
  (xor %A, (v4i1 VectorOfAllOnes)).

Added test setcc-combine.ll.

Thanks to Greg Bedwell for spotting this issue.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@232046 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/X86ISelLowering.cpp
test/CodeGen/X86/setcc-combine.ll [new file with mode: 0644]

index 2f236586e028f5b95dfab9468d5b9088442796c8..366e4b9e073477c2fdd54f0918182210ae6e2dce 100644 (file)
@@ -23178,45 +23178,51 @@ static SDValue PerformISDSETCCCombine(SDNode *N, SelectionDAG &DAG,
   if ((CC == ISD::SETNE || CC == ISD::SETEQ) && LHS.getOpcode() == ISD::SUB)
     if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(LHS.getOperand(0)))
       if (C->getAPIntValue() == 0 && LHS.hasOneUse()) {
-        SDValue addV = DAG.getNode(ISD::ADD, SDLoc(N),
-                                   LHS.getValueType(), RHS, LHS.getOperand(1));
-        return DAG.getSetCC(SDLoc(N), N->getValueType(0),
-                            addV, DAG.getConstant(0, addV.getValueType()), CC);
+        SDValue addV = DAG.getNode(ISD::ADD, SDLoc(N), LHS.getValueType(), RHS,
+                                   LHS.getOperand(1));
+        return DAG.getSetCC(SDLoc(N), N->getValueType(0), addV,
+                            DAG.getConstant(0, addV.getValueType()), CC);
       }
   if ((CC == ISD::SETNE || CC == ISD::SETEQ) && RHS.getOpcode() == ISD::SUB)
     if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS.getOperand(0)))
       if (C->getAPIntValue() == 0 && RHS.hasOneUse()) {
-        SDValue addV = DAG.getNode(ISD::ADD, SDLoc(N),
-                                   RHS.getValueType(), LHS, RHS.getOperand(1));
-        return DAG.getSetCC(SDLoc(N), N->getValueType(0),
-                            addV, DAG.getConstant(0, addV.getValueType()), CC);
+        SDValue addV = DAG.getNode(ISD::ADD, SDLoc(N), RHS.getValueType(), LHS,
+                                   RHS.getOperand(1));
+        return DAG.getSetCC(SDLoc(N), N->getValueType(0), addV,
+                            DAG.getConstant(0, addV.getValueType()), CC);
       }
 
-  if (VT.getScalarType() == MVT::i1) {
-    bool IsSEXT0 = (LHS.getOpcode() == ISD::SIGN_EXTEND) &&
-      (LHS.getOperand(0).getValueType().getScalarType() ==  MVT::i1);
-    bool IsVZero0 = ISD::isBuildVectorAllZeros(LHS.getNode());
-    if (!IsSEXT0 && !IsVZero0)
-      return SDValue();
-    bool IsSEXT1 = (RHS.getOpcode() == ISD::SIGN_EXTEND) &&
-      (RHS.getOperand(0).getValueType().getScalarType() ==  MVT::i1);
+  if (VT.getScalarType() == MVT::i1 &&
+      (CC == ISD::SETNE || CC == ISD::SETEQ || ISD::isSignedIntSetCC(CC))) {
+    bool IsSEXT0 =
+        (LHS.getOpcode() == ISD::SIGN_EXTEND) &&
+        (LHS.getOperand(0).getValueType().getScalarType() == MVT::i1);
     bool IsVZero1 = ISD::isBuildVectorAllZeros(RHS.getNode());
 
-    if (!IsSEXT1 && !IsVZero1)
-      return SDValue();
+    if (!IsSEXT0 || !IsVZero1) {
+      // Swap the operands and update the condition code.
+      std::swap(LHS, RHS);
+      CC = ISD::getSetCCSwappedOperands(CC);
+
+      IsSEXT0 = (LHS.getOpcode() == ISD::SIGN_EXTEND) &&
+                (LHS.getOperand(0).getValueType().getScalarType() == MVT::i1);
+      IsVZero1 = ISD::isBuildVectorAllZeros(RHS.getNode());
+    }
 
     if (IsSEXT0 && IsVZero1) {
-      assert(VT == LHS.getOperand(0).getValueType() && "Uexpected operand type");
-      if (CC == ISD::SETEQ)
+      assert(VT == LHS.getOperand(0).getValueType() &&
+             "Uexpected operand type");
+      if (CC == ISD::SETGT)
+        return DAG.getConstant(0, VT);
+      if (CC == ISD::SETLE)
+        return DAG.getConstant(1, VT);
+      if (CC == ISD::SETEQ || CC == ISD::SETGE)
         return DAG.getNOT(DL, LHS.getOperand(0), VT);
+      
+      assert((CC == ISD::SETNE || CC == ISD::SETLT) &&
+             "Unexpected condition code!");
       return LHS.getOperand(0);
     }
-    if (IsSEXT1 && IsVZero0) {
-      assert(VT == RHS.getOperand(0).getValueType() && "Uexpected operand type");
-      if (CC == ISD::SETEQ)
-        return DAG.getNOT(DL, RHS.getOperand(0), VT);
-      return RHS.getOperand(0);
-    }
   }
 
   return SDValue();
diff --git a/test/CodeGen/X86/setcc-combine.ll b/test/CodeGen/X86/setcc-combine.ll
new file mode 100644 (file)
index 0000000..c6ad5e0
--- /dev/null
@@ -0,0 +1,166 @@
+; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=generic < %s | FileCheck %s
+
+define i32 @test_eq_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_eq_1:
+; CHECK: pcmpgtd %xmm0, %xmm1
+; CHECK-NEXT: pxor {{.*}}(%rip), %xmm1
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp eq <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_ne_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_ne_1:
+; CHECK: pcmpgtd %xmm0, %xmm1
+; CHECK-NOT: pxor
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp ne <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_le_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_le_1:
+; CHECK: movl $-1, %eax
+; CHECK-NEXT: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sle <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_ge_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_ge_1:
+; CHECK: pcmpgtd %xmm0, %xmm1
+; CHECK: pxor {{.*}}(%rip), %xmm1
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sge <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_lt_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_lt_1:
+; CHECK: pcmpgtd %xmm0, %xmm1
+; CHECK-NOT: pxor
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp slt <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_gt_1(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_gt_1:
+; CHECK: xorl %eax, %eax
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %A, %B
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sgt <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_eq_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_eq_2:
+; CHECK: pcmpgtd %xmm1, %xmm0
+; CHECK-NEXT: pxor {{.*}}(%rip), %xmm0
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp eq <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_ne_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_ne_2:
+; CHECK: pcmpgtd %xmm1, %xmm0
+; CHECK-NOT: pxor
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp ne <4 x i32> %sext, zeroinitializer
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_le_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_le_2:
+; CHECK: pcmpgtd %xmm1, %xmm0
+; CHECK: pxor {{.*}}(%rip), %xmm0
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sle <4 x i32> zeroinitializer, %sext
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_ge_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_ge_2:
+; CHECK: movl $-1, %eax
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sge <4 x i32> zeroinitializer, %sext
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_lt_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_lt_2:
+; CHECK: pcmpgtd %xmm1, %xmm0
+; CHECK-NOT: pxor
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp slt <4 x i32> zeroinitializer, %sext
+  %0 = extractelement <4 x i1> %cmp, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}
+
+define i32 @test_gt_2(<4 x i32> %A, <4 x i32> %B) {
+; CHECK-LABEL: test_gt_2:
+; CHECK: pcmpgtd %xmm1, %xmm0
+; CHECK-NOT: pxor
+; CHECK: retq
+entry:
+  %cmp = icmp slt <4 x i32> %B, %A
+  %sext = sext <4 x i1> %cmp to <4 x i32>
+  %cmp1 = icmp sgt <4 x i32> zeroinitializer, %sext
+  %0 = extractelement <4 x i1> %cmp1, i32 1
+  %1 = sext i1 %0 to i32
+  ret i32 %1
+}