[PATCH] D142542: [InstSimplify] Simplify icmp between Shl instructions of the same value

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/test/Transforms/InstSimplify/compare.ll:2918
+
+define i1 @neg_icmp_lshr_known_non_zero_sgt_nuw_nsw_smallest_shift(i8 %x) {
+; CHECK-LABEL: @neg_icmp_lshr_known_non_zero_sgt_nuw_nsw_smallest_shift(
----------------
sdesmalen wrote:
> nit: this name is a bit confusing, I'm not sure what you mean with 'nuw_nsw_smallest_shift' ?
The idea here was that based on https://alive2.llvm.org/ce/z/YCbQCH, only the larger shift value needed to have nuw/(nuw && nsw) flags on it for the combine to be valid. However it's probably easier and more versatile to do the more generic combine (https://alive2.llvm.org/ce/z/tSyv_m), so this test can be replaced with something more appropriate


================
Comment at: llvm/test/Transforms/InstSimplify/compare.ll:2933
+
+define i1 @neg_icmp_lshr_different_shift_values() {
+; CHECK-LABEL: @neg_icmp_lshr_different_shift_values(
----------------
sdesmalen wrote:
> Can you just use `%x` and `%y` instead of `@llvm.vscale` here?
> 
> It would be good also to have at least one positive test with `@llvm.vscale()`, could you please add one?
Sure, I will add this and move the tests into a different patch and pre-commit them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142542