[PATCH] D66057: [InstCombine] Shift amount reassociation in bittest: trunc-of-shl (PR42399)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

Thanks for taking a look!



================
Comment at: llvm/test/Transforms/InstCombine/shift-amount-reassociation-in-bittest-with-truncation-shl.ll:414
   %t1 = lshr i32 %x, %t0
-  %t2 = add i32 %len, 1 ; too much
+  %t2 = add i32 %len, 32 ; too much
   %t2_wide = zext i32 %t2 to i64
----------------
spatel wrote:
> Please explain and/or add a comment here.
> If we leave the add constant as 1, this gets simplified away completely with this patch? Why doesn't it happen with 32? 
> Do we already have a test that covers the case for the original test (with add of 1)?
With `%t2 = add i32 %len, 1` the total shift is `((32-len)+(len+1))`==`33`.
The widest type here is `i64`, and 33 is a valid shift amount there.
But since one of the values being shifted is i32, the result can be constant-folded to false
since you will effectively shift-out all the bits.
https://rise4fun.com/Alive/kk1

With 32 here, the total shift amount will be 64, which is not okay for i64.
But that would already imply UB, and we **should** be folding it into `undef`
https://rise4fun.com/Alive/Tzn

I'm trying to keep changes reviewable, so i did not get to that part yet.

The `@t9_oneuse5` gets constant-folded too, so there is some coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66057