[PATCH] D22247: [DAGCombine] Make sext(setcc) combine respect getBooleanContents()

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Thanks, Matt!


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6208
@@ -6205,1 +6207,3 @@
     unsigned ElementWidth = VT.getScalarType().getSizeInBits();
+    unsigned SetCCWidth = N0.getValueType().getScalarType().getSizeInBits();
+
----------------
arsenm wrote:
> getScalarSizeInBits
Right, thanks!

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6211-6215
@@ -6206,3 +6210,7 @@
     SDLoc DL(N);
-    SDValue NegOne =
-      DAG.getConstant(APInt::getAllOnesValue(ElementWidth), DL, VT);
+    APInt TrueInt =
+        ((SetCCWidth != 1) && (TLI.getBooleanContents(VT) ==
+                               TargetLowering::ZeroOrOneBooleanContent))
+            ? APInt(ElementWidth, 1)
+            : APInt::getAllOnesValue(ElementWidth);
+    SDValue TrueVal = DAG.getConstant(TrueInt, DL, VT);
----------------
arsenm wrote:
> TLI should have a getConstantTrue(VT) helper (I thought I added this a long time ago but I don't see it)
I couldn't find one either. We have an isConstTrueVal(), but not a getConstTrueVal() (I think).
Do you think this is generally useful? If so, I can factor it out into TLI.

================
Comment at: test/CodeGen/X86/pr28504.ll:2-3
@@ +1,4 @@
+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
----------------
arsenm wrote:
> Usually just the triple is set on the RUN line
We have examples of both styles, for when we don't care about differences between triples:
test/CodeGen/X86$ grep 'target triple' *.ll | wc -l
483

Anyway, I can change this if you prefer.

================
Comment at: test/CodeGen/X86/pr28504.ll:10
@@ +9,3 @@
+; CHECK: sete  %cl
+define i32 @main() {
+bb:
----------------
arsenm wrote:
> This testcase can probably be reduced
Chandler said on the PR that he was unable to reduce it, so I haven't really tried to. I can see if there's a simpler way to trigger this.


http://reviews.llvm.org/D22247