[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,
----------------
mkuper wrote:
> chandlerc wrote:
> > 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....
> We're combining a (sext (setcc (...)).
> 
> There are two options:
> 1) The setcc's type is i1. So the sext of the "true" value is all-1s, regardless of getBooleanContents().
> 2) The setcc's type is something else, say, an i8. In this case, it has already been extended - either zext, or sext, according to getBooleanContents(). So to sext it further, we want to splat the highest bit of the i8. For the "true" case, the result of this sext is the getBooleanContents-respecting true value of the right width.
> 
> I'll document this inline.
Ah, I guess I see.

Maybe call this ExtTrueValue then to clarify that in the i1 case it may not match the TrueVal of the given size?


https://reviews.llvm.org/D22247