[PATCH] D49179: [InstCombine] Fold x & (-1 >> y) == x to x u<= (-1 >> y)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4447-4449
+static Value *
+canonicalizeUnsignedTruncationCheckLikeICmps(ICmpInst &I,
+                                             InstCombiner::BuilderTy &Builder) {
----------------
spatel wrote:
> We're also missing signed folds like:
> 
> ```
> Pre: isPowerOf2(C1+1)
> %masked = and i32 %arg, C1
> %truncheck = icmp sge i32 %masked, %arg
>   =>
> %truncheck = icmp sle i32 %arg, C1
> 
> ```
> https://rise4fun.com/Alive/K8H
> 
> ...so I'd give this function a more general name: foldICmpWithMaskedVal?
Yep, i didn't like that long name anyway.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4452-4458
+  if (!match(&I, m_c_ICmp(SrcPred,
+                          m_c_And(m_CombineAnd(m_CombineOr(m_LShr(m_AllOnes(),
+                                                                  m_Value()),
+                                                           m_LowBitMask()),
+                                               m_Value(M)),
+                                  m_Value(X)),
+                          m_Deferred(X))))
----------------
spatel wrote:
> Untested, but this could be less indent-crazy if we give some of it a local name like:
> 
>   auto m_Mask = m_CombineOr(m_LShr(m_AllOnes(), m_Value()), m_LowBitMask());
>   if (!match(&I, m_c_ICmp(SrcPred,
>                           m_c_And(m_CombineAnd(m_Mask, m_Value(M)), m_Value(X)),
>                           m_Deferred(X))))
> 
Ha, nice one.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4659-4660
 
   if (Instruction *Res = foldICmpBinOp(I))
     return Res;
 
----------------
spatel wrote:
> There's no logic to fold ordering within visitICmpInst()...the new function could be called from within foldICmpBinOp() rather than the top-level?
Hm, good idea.


Repository:
  rL LLVM

https://reviews.llvm.org/D49179