[PATCH] D107766: [AggressiveInstCombine] Add `lshr` and `ashr` instructions to TruncInstCombine DAG

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

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

>> 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()`.
>
> Hmm, how could we compute knownbits of the shift amount at compile time? Do you mean analyzing DAG for the shift amount Value, taking knowbits recursively?

You've seen `llvm::computeKnownBits()`, right?

> Also I don't believe this computing makes sense: for the most cases, when shift amount is variable, its first byte is unknown. For instance, how could knownbits help to optimize @spatel's example?
>
>   define i16 @sh_amt(i8 %x, i8 %sh1) {
>     %z = zext i8 %x to i32
>     %zs = zext i8 %sh1 to i32
>     %s = lshr i32 %z, %zs
>     %a = add nuw nsw i32 %s, %z
>     %s2 = lshr i32 %a, 2
>     %t = trunc i32 %s2 to i16
>     ret i16 %t
>   }

I find this comment to be highly inflammatory.

Just because there's large number of cases it won't help doesn't mean it can't ever help with anything.
https://alive2.llvm.org/ce/z/RkkBTy <- we have no idea what `%y` is, but we can tell it's less than the target bitwidth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107766