[PATCH] D25913: [InstCombine] Fold nuw left-shifts in `ugt`/`ule` comparisons.

[bryant via llvm-commits]

bryant marked 3 inline comments as done.
bryant added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1960
+    if (X->getType()->isVectorTy())
+      CTy = VectorType::get(CTy, X->getType()->getVectorNumElements());
+    // Derivation for the ult case:
----------------
efriedma wrote:
> bryant wrote:
> > efriedma wrote:
> > > Isn't CTy always equal to "X->getType()"?
> > Since we're morphing `(x << s) ugt c` into `x ugt (c >> s)`, CTy needs to be enough to hold `c >> s`.
> > 
> > `c`'s type would be sufficient thanks to the right shift.
> > 
> > But unlike other binops, shift nodes place no constraint between the types of shiftee and shift amount (I'm really just inferring from my memory of TargetSelectionDAG.td, so I could be wrong).
> > 
> > So X-getType() might not be wide enough to hold `c >> s`.
> In IR the two operands of an shl always have the same type.  (The rules change once you get to SelectionDAG.)
You're right, just checked LangRef. And on top of that, icmp obviously needs both sides to be the same. Fixed.


Repository:
  rL LLVM

https://reviews.llvm.org/D25913