[PATCH] D126617: [InstCombine] Optimize shl+lshr+and conversion pattern

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added a comment.

Sorry for the late response. Based on the discussion I have some questions:

1. the demanded bits fix. I think it is a safe and independent change. Should I check in this part first ? Or @spatel can you help to do this as the author is you.

  diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  index 278db05f65d1..c0d92fc27bb6 100644
  --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  @@ -630,6 +630,18 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
               ComputeNumSignBits(I->getOperand(0), Depth + 1, CxtI);
           if (SignBits >= NumHiDemandedBits)
             return I->getOperand(0);
  +
  +        // If we can pre-shift a left-shifted constant to the right without
  +        // losing any low bits (we already know we don't demand the high bits):
  +        // (C << X) >> SA --> (C >> SA) << X
  +        Value *X;
  +        const APInt *C;
  +        if (match(I->getOperand(0), m_Shl(m_APInt(C), m_Value(X))) &&
  +            C->countTrailingZeros() >= ShiftAmt) {
  +          Constant *ShiftC = ConstantInt::get(I->getType(), C->lshr(ShiftAmt));
  +          Instruction *Shl = BinaryOperator::CreateShl(ShiftC, X);
  +          return InsertNewInstWith(Shl, *I);
  +        }
         }

2. Should we fix  'shift+and' to 'icmp+select' transform? I think backend is not easy to revert the transform because it is not correct based on alive2(https://alive2.llvm.org/ce/z/MgNqEW) if these is no other condition.
3. How to fix @shl_lshr_pow2_const. I think add the larger pattern match for and(shift(shift)) should be the diff5 in this review.
4. Do we need D126591 <https://reviews.llvm.org/D126591> after we fix @shl_lshr_pow2_const? It still have some patterns we can't covered by any solution we discussed on this review like `((C1 << X) & C2) != 0 -> X  < (Log2(C2+C1) - Log2(C1))`



================
Comment at: llvm/test/Transforms/InstCombine/and.ll:1625
 
 define i16 @shl_lshr_pow2_const(i16 %x) {
 ; CHECK-LABEL: @shl_lshr_pow2_const(
----------------
spatel wrote:
> This diff does not exist with the current test on "main", right? Is this review baselined against another patch?
I'm sorry this is based on Diff3. I will rebase the review based on main.


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

https://reviews.llvm.org/D126617