[PATCH] D142783: [InstCombine][LV] Fold (add (zext (add X, -1)), 1) -> (zext X) if X is non-zero.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 14:33:40 PST 2023


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2877
+
+define i32 @dec_zext_add_assume_nonzero(i8 %x) {
+; CHECK-LABEL: @dec_zext_add_assume_nonzero(
----------------
craig.topper wrote:
> I will pre-commit these if they look ok. I wasn't sure if this was the right file or why we have add2.ll, add3.ll, add4.ll
I'm not sure what differentiates those test files either. We could use a lot of test dir cleanup in general, but it never seems to be high enough priority.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2901
+  call void @llvm.assume(i1 %z)
+  %a = sub i8 %x, 1
+  %b = zext i8 %a to i32
----------------
This will get canonicalized to `add`, or many other things will break. So there's not much value in this particular test IMO.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2927
+  %o = or <2 x i8> %x, <i8 8, i8 8>
+  %a = add <2 x i8> %o, <i8 -1, i8 -1>
+  %b = zext <2 x i8> %a to <2 x i32>
----------------
It seems unlikely to matter in practice, but I think the vector constant matching is inconsistent. We'll allow poison elements in the -1, but not the +1? Could add a test or two to show that gap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142783



More information about the llvm-commits mailing list