[PATCH] D155095: TargetLowering: fix an infinite DAG combine in SimplifySETCC

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4237
   ISD::CondCode SwappedCC = ISD::getSetCCSwappedOperands(Cond);
-  if (N0ConstOrSplat && (!OpVT.isScalableVector() || !N1ConstOrSplat) &&
+  if (N0ConstOrSplat && ((!OpVT.isScalableVector() && N0Splat && !N1Splat) ||
+                         !N1ConstOrSplat) &&
----------------
craig.topper wrote:
> craig.topper wrote:
> > I'm not sure about the logic of this carve out. splat_vector is primarily used with scalable vectors. So checking it is not a scalable vector, but N0 is a splat_vector seems odd.
> I think the check you want is just `N0ConstOrSplat && !N1ConstOrSplat`. The old code only checked !N1ConstOrSplat for scalable vectors. Now you want to check it regardless of scalable vectors.
> I think the check you want is just N0ConstOrSplat && !N1ConstOrSplat.


That's what the landed-but-reverted commit [1] did, but it broke some RISCV tests that IIUC are expecting that `(setcc (splat (const)), (buildvector (const)))` get normalized to put the constant splat on the RHS (so that subsequent combine(s) can then constant-fold that, I think?).

1: https://reviews.llvm.org/D155095?id=539663


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155095/new/

https://reviews.llvm.org/D155095