[PATCH] D63829: [InstCombine] Shift amount reassociation in bittest (PR42399)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3267-3277
+  if (!match(
+          &I,
+          m_ICmp(Pred,
+                 m_OneUse(m_c_And(
+                     m_OneUse(m_CombineAnd(
+                         m_LogicalShift(m_Value(X), m_Value(XShAmt)),
+                         m_Value(XShift))),
----------------
spatel wrote:
> This would be easier to read for me if we did the simple icmp checks first:
>   if (!I.isEquality() || !match(I.getOperand(1), m_Zero()))
>     return nullptr;
> 
> And then do a mega-match on only I.getOperand(0).
> 
> ...although that match raises a question if the one-use checks are balanced:
> Do we prefer to shift left or right?
> https://rise4fun.com/Alive/
>   %t0 = lshr i32 %x, 2
>   %t1 = and i32 %t0, %y
>   %t3 = icmp ne i32 %t1, 0
>     =>
>   %v0 = shl i32 %y, 2
>   %v1 = and i32 %v0, %x
>   %t3 = icmp ne i32 %v1, 0
Incomplete Alive link in previous comment:
https://rise4fun.com/Alive/9Kp


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63829