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

Michael Kuperstein via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 12 11:39:34 PDT 2016


mkuper added inline comments.

================
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:
> mkuper wrote:
> > 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.
> Yes, there are plenty of places that have broken BooleanContent handling for targets that use -1, so more helpers would be useful
Actually, this is a bit problematic here. I could add a helper for getConstTrueVal, but I'd still need to special-case (SetCCWidth == 1), since in that case, I really want the sign-extension of the low bit, regardless of getBooleanContents(). So it doesn't really make the code much less cumbersome. I'll post a version of the patch with that.


http://reviews.llvm.org/D22247





More information about the llvm-commits mailing list