[PATCH] D107766: [InstCombine] Get rid of `hasOneUses()` when swapping `lshr` and `zext`

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D107766#2937021 <https://reviews.llvm.org/D107766#2937021>, @anton-afanasyev wrote:

> In D107766#2936935 <https://reviews.llvm.org/D107766#2936935>, @lebedev.ri wrote:
>
>> Some observations for logical right-shift.
>> this will have a hard time with variable shift amounts.
>> You need to avoid creating out-of-bounds shifts, there are two obvious options:
>>
>> 1. https://alive2.llvm.org/ce/z/XShcju <- shift amount needs to be less than target width, and truncation should only drop zeros.
>> 2. https://alive2.llvm.org/ce/z/QiDPV7 <- could saturate the shift amount if you know that `%x` has more leading zeros than the number of bits to be truncated
>>
>> We might already have this logic somewhere, not sure.
>
> Yes, thanks for your observations, I'm already working on it: https://alive2.llvm.org/ce/z/XcCJ9Q
> There is also special care for the vector case to make a transform not being more poisonous.
> `TruncInstCombine` already has appropriate logic but needs to be tweaked. 
> For now I'm supposing that shift amout is constant (int or vector).



> Not sure that transform adding check for variable shift amount is good.

Define good. I think supporting variable shifts will take exactly two lines:
compute knownbits of the shift amount, and get the maximal shift amount via `KnownBits::getMaxValue()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107766