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

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added a comment.

In D126617#3547933 <https://reviews.llvm.org/D126617#3547933>, @spatel wrote:

> In D126617#3547436 <https://reviews.llvm.org/D126617#3547436>, @bcl5980 wrote:
>
>> In D126617#3547120 <https://reviews.llvm.org/D126617#3547120>, @spatel wrote:
>>
>>> I still think we should split this patch up as 2 independent transforms.
>>>
>>> The opposite shifts transform doesn't seem like it should be a power-of-2-mask transform. Can we handle that using demanded bits instead? Double-check (you can pre-commit more tests as needed), but I don't think this patch will handle these related folds:
>>> https://alive2.llvm.org/ce/z/SNmj5M
>>
>> Thanks for the mention. Is this transform you want ?
>> https://alive2.llvm.org/ce/z/-C8L9U
>> If yes, I will send a new patch to do this.
>
> Yes, the first pre-condition looks correct. We don't actually care what the final instruction in the sequence is - it just has to remove demand of the high bits. The last instruction could be a trunc for example, so we should have tests with that too:
> https://alive2.llvm.org/ce/z/ZCgqj5
>
> We already look for that pattern in InstCombine's demanded bits. So I think we just need to add a transform like this:
>
>   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);
>   +        }
>          }
>    
>          // Unsigned shift right.

Wow, that's cool. A very general solution.


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

https://reviews.llvm.org/D126617