[PATCH] D42948: [SelectionDAG] Support some SimplifySetCC cases for comparing against vector splats of constants.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1891
       if (C1 == MinVal)
-        return DAG.getConstant(1, dl, VT);
-
-      // X >= C0 --> X > (C0 - 1)
-      APInt C = C1 - 1;
-      ISD::CondCode NewCC = (Cond == ISD::SETGE) ? ISD::SETGT : ISD::SETUGT;
-      if ((DCI.isBeforeLegalizeOps() ||
-           isCondCodeLegal(NewCC, VT.getSimpleVT())) &&
-          (!N1C->isOpaque() || (N1C->isOpaque() && C.getBitWidth() <= 64 &&
-                                isLegalICmpImmediate(C.getSExtValue())))) {
-        return DAG.getSetCC(dl, VT, N0,
-                            DAG.getConstant(C, dl, N1.getValueType()),
-                            NewCC);
+        return DAG.getBooleanTrueConstant(dl, VT);
+
----------------
spatel wrote:
> spatel wrote:
> > craig.topper wrote:
> > > spatel wrote:
> > > > Is this (and similar diffs below) currently wrong for ZeroOrNegativeOneBooleanContent targets?
> > > > I'm having a hard time seeing all the pre-conditions in this 900 line monster of a function...
> > > I suspect it is. I believe there are other lines in here that I didn't touch that are probably also wrong.
> > > 
> > > I can split out the getBooleanTrueConstant function and fix this up for scalar types first if you want?
> > If it's not too hard, that would be good. A quick check of in-tree targets says AMDGPU, NVPTX, and some flavors of MIPS could hit that bug via scalar ZeroOrNegativeOneBooleanContent.
> Oops - I read that wrong. Looks like all MIPS have ZeroOrOneBooleanContent for scalars.
In most cases we probably optimize the setcc before type legalization so the result type is i1 making the difference not meaningful. So to hit a bug you'd need to only see the simplifcation after the result type has been promoted.


Repository:
  rL LLVM

https://reviews.llvm.org/D42948