[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