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

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6210-6216
@@ -6206,4 +6209,9 @@
     SDLoc DL(N);
-    SDValue NegOne =
-      DAG.getConstant(APInt::getAllOnesValue(ElementWidth), DL, VT);
+    SDValue TrueVal =
+        (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), TrueVal,
----------------
I'm confused....

Why wouldn't all this special casing of size one still be sunk into getConstTrueVal? I feel like I should understand this from your previous comment, but perhaps I'm being dense and don't....

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6211-6214
@@ +6210,6 @@
+    SDValue TrueVal =
+        (SetCCWidth == 1)
+            ? DAG.getConstant(APInt::getAllOnesValue(VT.getScalarSizeInBits()),
+                              DL, VT)
+            : TLI.getConstTrueVal(DAG, VT, DL);
+
----------------
Why not sink all of this logic into getConstTrueVal?

================
Comment at: test/CodeGen/X86/pr28504.ll:3-4
@@ +2,4 @@
+
+; CHECK-LABEL: main:
+; CHECK: movb  $1, %al
+; CHECK: xorl  %ecx, %ecx
----------------
Moving the triple to the RUN line is preferred at this point.

================
Comment at: test/CodeGen/X86/pr28504.ll:11
@@ +10,3 @@
+  %tmp1 = alloca i8, align 1
+  %tmp2 = alloca i32, align 4
+  %tmp3 = alloca i16, align 2
----------------
You can probably reduce it much more now that you know what to look for. I was just shooting blind. =]


https://reviews.llvm.org/D22247