[PATCH] D44997: [InstCombine] Fold compare of int constant against a splatted vector of ints

[Daniel Neilson via Phabricator via llvm-commits]

dneilson added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2478-2483
+      // Check whether every element of Mask is the same constant
+      ConstantInt *Elem = nullptr;
+      if (isa<ConstantAggregateZero>(Mask)) // zeroinitializer
+        Elem = Builder.getInt32(0);
+      else if (auto *CV = dyn_cast<ConstantDataVector>(Mask))
+        Elem = dyn_cast_or_null<ConstantInt>(CV->getSplatValue());
----------------
spatel wrote:
> Constant::getSplatValue() ?
Completely missed that interface. Thank you! I was not happy with the gymnastics I had to do here; I'm glad there's a better way.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2497-2501
+          // Which vector to extract from?
+          Value *Vec = Vec1;
+          const APInt &ElemVal = Elem->getValue();
+          if (ElemVal.uge(VecTy->getNumElements())) {
+            Vec = Vec2;
----------------
spatel wrote:
> None of this is necessary AFAIK. A splat shuffle will be canonicalized so that the 2nd operand is undef and the mask constant is adjusted to choose from the 1st operand.
Right you are. I had in my head, for some reason, that it wasn't getting simplified.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2509-2512
+          // If C isn't a pattern, and the pred is for (in)equality
+          // then we can fold the icmp into true/false.
+          Return = (Pred == ICmpInst::ICMP_NE) ? Builder.getTrue()
+                                               : Builder.getFalse();
----------------
spatel wrote:
> Probably better to handle this part independently in InstSimplify (if it really matters?).
Seemed like a nice bonus; we've already done all of the heavy lifting doing the match and such here. Replicating that into inst-simplify doesn't seem worth the bother.


Repository:
  rL LLVM

https://reviews.llvm.org/D44997