[DAGCombine] Make sext(setcc) combine respect getBooleanContents

We used to combine "sext(setcc x, y, cc) -> (select (setcc x, y, cc), -1, 0)"
Instead, we should combine to (select (setcc x, y, cc), T, 0) where the value
of T is 1 or -1, depending on the type of the setcc, and getBooleanContents()
for the type if it is not i1.

This fixes PR28504.

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

diff --git a/include/llvm/Target/TargetLowering.h b/include/llvm/Target/TargetLowering.h
index d21d321..4586a17 100644 (file)
--- a/include/llvm/Target/TargetLowering.h
+++ b/include/llvm/Target/TargetLowering.h
@@ -2349,6 +2349,10 @@ public:
   /// from getBooleanContents().
   bool isConstFalseVal(const SDNode *N) const;
 
+  /// Return a constant of type VT that contains a true value that respects
+  /// getBooleanContents()
+  SDValue getConstTrueVal(SelectionDAG &DAG, EVT VT, const SDLoc &DL) const;
+
   /// Return if \p N is a True value when extended to \p VT.
   bool isExtendedTrueVal(const ConstantSDNode *N, EVT VT, bool Signed) const;
 

diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index b8afe41..6a74430 100644 (file)
--- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -6198,13 +6198,27 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
       }
     }
 
-    // sext(setcc x, y, cc) -> (select (setcc x, y, cc), -1, 0)
-    unsigned ElementWidth = VT.getScalarType().getSizeInBits();
+    // sext(setcc x, y, cc) -> (select (setcc x, y, cc), T, 0)
+    // Here, T can be 1 or -1, depending on the type of the setcc and
+    // getBooleanContents().
+    unsigned SetCCWidth = N0.getValueType().getScalarSizeInBits();
+
     SDLoc DL(N);
-    SDValue NegOne =
-      DAG.getConstant(APInt::getAllOnesValue(ElementWidth), DL, VT);
+    // To determine the "true" side of the select, we need to know the high bit
+    // of the value returned by the setcc if it evaluates to true.
+    // If the type of the setcc is i1, then the true case of the select is just
+    // sext(i1 1), that is, -1.
+    // If the type of the setcc is larger (say, i8) then the value of the high
+    // bit depends on getBooleanContents(). So, ask TLI for a real "true" value
+    // of the appropriate width.
+    SDValue ExtTrueVal =
+        (SetCCWidth == 1)
+            ? DAG.getConstant(APInt::getAllOnesValue(VT.getScalarSizeInBits()),
+                              DL, VT)
+            : TLI.getConstTrueVal(DAG, VT, DL);
+
     if (SDValue SCC = SimplifySelectCC(
-            DL, N0.getOperand(0), N0.getOperand(1), NegOne,
+            DL, N0.getOperand(0), N0.getOperand(1), ExtTrueVal,
             DAG.getConstant(0, DL, VT),
             cast<CondCodeSDNode>(N0.getOperand(2))->get(), true))
       return SCC;
@@ -6215,10 +6229,10 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
           TLI.isOperationLegal(ISD::SETCC, N0.getOperand(0).getValueType())) {
         SDLoc DL(N);
         ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
-        SDValue SetCC = DAG.getSetCC(DL, SetCCVT,
-                                     N0.getOperand(0), N0.getOperand(1), CC);
-        return DAG.getSelect(DL, VT, SetCC,
-                             NegOne, DAG.getConstant(0, DL, VT));
+        SDValue SetCC =
+            DAG.getSetCC(DL, SetCCVT, N0.getOperand(0), N0.getOperand(1), CC);
+        return DAG.getSelect(DL, VT, SetCC, ExtTrueVal,
+                             DAG.getConstant(0, DL, VT));
       }
     }
   }

diff --git a/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 3679810..bb67aa3 100644 (file)
--- a/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -1234,6 +1234,16 @@ bool TargetLowering::isConstTrueVal(const SDNode *N) const {
   llvm_unreachable("Invalid boolean contents");
 }
 
+SDValue TargetLowering::getConstTrueVal(SelectionDAG &DAG, EVT VT,
+                                        const SDLoc &DL) const {
+  unsigned ElementWidth = VT.getScalarSizeInBits();
+  APInt TrueInt =
+      getBooleanContents(VT) == TargetLowering::ZeroOrOneBooleanContent
+          ? APInt(ElementWidth, 1)
+          : APInt::getAllOnesValue(ElementWidth);
+  return DAG.getConstant(TrueInt, DL, VT);
+}
+
 bool TargetLowering::isConstFalseVal(const SDNode *N) const {
   if (!N)
     return false;

diff --git a/test/CodeGen/X86/pr28504.ll b/test/CodeGen/X86/pr28504.ll
new file mode 100644 (file)
index 0000000..a617c8a
--- /dev/null
+++ b/test/CodeGen/X86/pr28504.ll
@@ -0,0 +1,37 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+
+; The test case is rather involved, because we need to get to a state where
+; We have a sext(setcc x, y, cc) -> (select (setcc x, y, cc), T, 0) combine,
+; BUT this combine is only triggered post-legalization, so the setcc's return
+; type is i8. So we can't have the combine opportunity be exposed too early.
+; Basically, what we want to see is that the compare result zero-extended, and 
+; then stored. Only one zext, and no sexts.
+
+; CHECK-LABEL: main:
+; CHECK: movzbl (%rdi), %[[EAX:.*]]
+; CHECK-NEXT: xorl  %e[[C:.]]x, %e[[C]]x
+; CHECK-NEXT: cmpl  $1, %[[EAX]]
+; CHECK-NEXT: sete  %[[C]]l
+; CHECK-NEXT: movl  %e[[C]]x, (%rsi)
+define void @main(i8* %p, i32* %q) {
+bb:
+  %tmp4 = load i8, i8* %p, align 1
+  %tmp5 = sext i8 %tmp4 to i32
+  %tmp6 = load i8, i8* %p, align 1
+  %tmp7 = zext i8 %tmp6 to i32
+  %tmp8 = sub nsw i32 %tmp5, %tmp7
+  %tmp11 = icmp eq i32 %tmp7, 1
+  %tmp12 = zext i1 %tmp11 to i32
+  %tmp13 = add nsw i32 %tmp8, %tmp12
+  %tmp14 = trunc i32 %tmp13 to i8
+  %tmp15 = sext i8 %tmp14 to i16
+  %tmp16 = sext i16 %tmp15 to i32
+  store i32 %tmp16, i32* %q, align 4
+  br i1 %tmp11, label %bb21, label %bb22
+
+bb21:                                             ; preds = %bb
+  unreachable
+
+bb22:                                             ; preds = %bb
+  ret void
+}