From: Sanjay Patel <spatel@rotateright.com>
Date: Fri, 23 Jun 2017 18:42:15 +0000 (+0000)
Subject: [x86] fix value types for SBB transform (PR33560)
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=243c5c12f5b579a3d316dadffe0d238aa8f8f658;p=llvm

[x86] fix value types for SBB transform (PR33560)

I'm not sure yet why this wouldn't fail in the simple case,
but clearly I used the wrong value type with:
https://reviews.llvm.org/rL306040

...and the bug manifests with:
https://bugs.llvm.org/show_bug.cgi?id=33560



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

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index cf88673351e..0be5460fa34 100644
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -34898,7 +34898,7 @@ static SDValue combineAddOrSubToADCOrSBB(SDNode *N, SelectionDAG &DAG) {
     return SDValue();
 
   SDValue Z = Cmp.getOperand(0);
-  SDVTList VTs = DAG.getVTList(N->getValueType(0), MVT::i32);
+  EVT ZVT = Z.getValueType();
 
   // If X is -1 or 0, then we have an opportunity to avoid constants required in
   // the general case below.
@@ -34909,19 +34909,21 @@ static SDValue combineAddOrSubToADCOrSBB(SDNode *N, SelectionDAG &DAG) {
     // -1 + (Z == 0) --> sbb %eax, %eax, (neg Z)
     if ((IsSub && CC == X86::COND_NE && ConstantX->isNullValue()) ||
         (!IsSub && CC == X86::COND_E && ConstantX->isAllOnesValue())) {
-      SDValue Zero = DAG.getConstant(0, DL, VT);
-      SDValue Neg = DAG.getNode(X86ISD::SUB, DL, VTs, Zero, Z);
+      SDValue Zero = DAG.getConstant(0, DL, ZVT);
+      SDVTList X86SubVTs = DAG.getVTList(ZVT, MVT::i32);
+      SDValue Neg = DAG.getNode(X86ISD::SUB, DL, X86SubVTs, Zero, Z);
       return DAG.getNode(X86ISD::SETCC_CARRY, DL, VT,
                          DAG.getConstant(X86::COND_B, DL, MVT::i8),
                          SDValue(Neg.getNode(), 1));
     }
+
     // cmp with 1 sets the carry flag when Z == 0, so create 0 or -1 using 'sbb'
     // with fake operands:
     //  0 - (Z == 0) --> sbb %eax, %eax, (cmp Z, 1)
     // -1 + (Z != 0) --> sbb %eax, %eax, (cmp Z, 1)
     if ((IsSub && CC == X86::COND_E && ConstantX->isNullValue()) ||
         (!IsSub && CC == X86::COND_NE && ConstantX->isAllOnesValue())) {
-      SDValue One = DAG.getConstant(1, DL, Z.getValueType());
+      SDValue One = DAG.getConstant(1, DL, ZVT);
       SDValue Cmp1 = DAG.getNode(X86ISD::CMP, DL, MVT::i32, Z, One);
       return DAG.getNode(X86ISD::SETCC_CARRY, DL, VT,
                          DAG.getConstant(X86::COND_B, DL, MVT::i8), Cmp1);
@@ -34929,19 +34931,22 @@ static SDValue combineAddOrSubToADCOrSBB(SDNode *N, SelectionDAG &DAG) {
   }
 
   // (cmp Z, 1) sets the carry flag if Z is 0.
-  SDValue NewCmp = DAG.getNode(X86ISD::CMP, DL, MVT::i32, Z,
-                               DAG.getConstant(1, DL, Z.getValueType()));
+  SDValue One = DAG.getConstant(1, DL, ZVT);
+  SDValue Cmp1 = DAG.getNode(X86ISD::CMP, DL, MVT::i32, Z, One);
+
+  // Add the flags type for ADC/SBB nodes.
+  SDVTList VTs = DAG.getVTList(VT, MVT::i32);
 
   // X - (Z != 0) --> sub X, (zext(setne Z, 0)) --> adc X, -1, (cmp Z, 1)
   // X + (Z != 0) --> add X, (zext(setne Z, 0)) --> sbb X, -1, (cmp Z, 1)
   if (CC == X86::COND_NE)
     return DAG.getNode(IsSub ? X86ISD::ADC : X86ISD::SBB, DL, VTs, X,
-                       DAG.getConstant(-1ULL, DL, VT), NewCmp);
+                       DAG.getConstant(-1ULL, DL, VT), Cmp1);
 
   // X - (Z == 0) --> sub X, (zext(sete  Z, 0)) --> sbb X, 0, (cmp Z, 1)
   // X + (Z == 0) --> add X, (zext(sete  Z, 0)) --> adc X, 0, (cmp Z, 1)
   return DAG.getNode(IsSub ? X86ISD::SBB : X86ISD::ADC, DL, VTs, X,
-                     DAG.getConstant(0, DL, VT), NewCmp);
+                     DAG.getConstant(0, DL, VT), Cmp1);
 }
 
 static SDValue combineLoopMAddPattern(SDNode *N, SelectionDAG &DAG,
diff --git a/test/CodeGen/X86/sbb.ll b/test/CodeGen/X86/sbb.ll
index 409af00d3b2..bc00fc7c66a 100644
--- a/test/CodeGen/X86/sbb.ll
+++ b/test/CodeGen/X86/sbb.ll
@@ -111,3 +111,28 @@ define i8 @i8_select_neg1_or_0_commuted_as_math(i8 %x) {
   ret i8 %add
 }
 
+; Make sure we're creating nodes with the right value types. This would crash.
+; https://bugs.llvm.org/show_bug.cgi?id=33560
+
+define void @PR33560(i8 %x, i64 %y) {
+; CHECK-LABEL: PR33560:
+; CHECK:       # BB#0: # %entry
+; CHECK-NEXT:    negb %dil
+; CHECK-NEXT:    sbbq %rax, %rax
+; CHECK-NEXT:    cmpq %rsi, %rax
+; CHECK-NEXT:    retq
+entry:
+  %cmp1 = icmp eq i8 %x, 0
+  %ext = zext i1 %cmp1 to i64
+  %add = add i64 %ext, -1
+  %cmp2 = icmp eq i64 %add, %y
+  br i1 %cmp2, label %end, label %else
+
+else:
+  %tmp7 = zext i1 %cmp1 to i8
+  br label %end
+
+end:
+  ret void
+}
+