[PATCH] D141412: [InstCombine]: Add tests for icmp ne non-zero power of 2; NFC

Tue Jan 10 12:45:03 PST 2023

nikic added a reviewer: spatel.
nikic added a comment.

Generally looks fine, but I would omit most of the tests on different bitwidths. Sometime we have one test for a different bitwidth (picking something weird like `i13`) to show that it doesn't matter, but otherwise testing just a single small bitwidth like `i8` or `i16` is enough (small bitwidth preferred to avoid alive2 timeouts). Not going to insist though.

I'm assuming the non-constant tests are there for the sake of completeness, and there is no intent to support them.

================
Comment at: llvm/test/Transforms/InstCombine/icmp-ne-pow2.ll:332
+  %ygt = icmp ugt i32 %y, %x
+  call void @llvm.assume(i1 %ygt)
+
----------------
Not sure I understand the significance of this test. It seems like the same as the previous with this extra assume, but why is it relevant?

================
Comment at: llvm/test/Transforms/InstCombine/icmp-ne-pow2.ll:354
+  %ctpop = call i32 @llvm.ctpop.i32(i32 %y)
+  %yp2 = icmp ne i32 %ctpop, 1
+  call void @llvm.assume(i1 %yp2)
----------------
Doing something like `ule 1` would be more meaningful, in that it is "nearly" pow2.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141412