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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D107766#2935073 <https://reviews.llvm.org/D107766#2935073>, @lebedev.ri wrote:

> Nice, this seems to fit naturally there.
> That being said, you probably still want some standalone tests for the pattern in question, both a positive ones, and a negative ones - what's the requirement on the shift amount?

Agree - aggressive-instcombine doesn't get nearly as much testing as regular instcombine, so we need more tests to be confident it doesn't over-reach.
Leaving the shift amount off of the getRelevantOperands() list doesn't work on this example (crash):

  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
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107766