Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstCombine] Canonicalize icmp ult (add X, C2), C expressions #95649

Merged

Conversation

antoniofrighetto
Copy link
Contributor

icmp ult (add X, C2), C can be folded to icmp ne (and X, C), 2C, subject to C == -C2 and C2 being a power of 2.

Proofs: https://alive2.llvm.org/ce/z/P-VVmQ.

Fixes: #75613.

@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Antonio Frighetto (antoniofrighetto)

Changes

icmp ult (add X, C2), C can be folded to icmp ne (and X, C), 2C, subject to C == -C2 and C2 being a power of 2.

Proofs: https://alive2.llvm.org/ce/z/P-VVmQ.

Fixes: #75613.


Full diff: https://github.com/llvm/llvm-project/pull/95649.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+7)
  • (modified) llvm/test/Transforms/InstCombine/icmp-add.ll (+65)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 34b0f8b860497..51dd3384c1fb5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -3130,6 +3130,13 @@ Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
     return new ICmpInst(ICmpInst::ICMP_EQ, Builder.CreateAnd(X, -C),
                         ConstantExpr::getNeg(cast<Constant>(Y)));
 
+  // X+C2 <u C -> (X & C) == 2C
+  //   iff C == -(C2)
+  //       C2 is a power of 2
+  if (Pred == ICmpInst::ICMP_ULT && C2->isPowerOf2() && (C == -(*C2)))
+    return new ICmpInst(ICmpInst::ICMP_NE, Builder.CreateAnd(X, C),
+                        ConstantInt::get(Ty, C * 2));
+
   // X+C >u C2 -> (X & ~C2) != C
   //   iff C & C2 == 0
   //       C2+1 is a power of 2
diff --git a/llvm/test/Transforms/InstCombine/icmp-add.ll b/llvm/test/Transforms/InstCombine/icmp-add.ll
index 6b4e5a5372c52..f5e8a58580ce8 100644
--- a/llvm/test/Transforms/InstCombine/icmp-add.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-add.ll
@@ -3023,4 +3023,69 @@ define i1 @icmp_addnuw_nonzero_fail_multiuse(i32 %x, i32 %y) {
   ret i1 %c
 }
 
+define i1 @ult_add_C2_pow2_C_neg(i8 %x) {
+; CHECK-LABEL: @ult_add_C2_pow2_C_neg(
+; CHECK-NEXT:    [[TMP1:%.*]] = and i8 [[X:%.*]], -32
+; CHECK-NEXT:    [[C:%.*]] = icmp ne i8 [[TMP1]], -64
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add i8 %x, 32
+  %c = icmp ult i8 %i, -32
+  ret i1 %c
+}
+
+define i1 @ult_add_nsw_C2_pow2_C_neg(i8 %x) {
+; CHECK-LABEL: @ult_add_nsw_C2_pow2_C_neg(
+; CHECK-NEXT:    [[TMP1:%.*]] = and i8 [[X:%.*]], -32
+; CHECK-NEXT:    [[C:%.*]] = icmp ne i8 [[TMP1]], -64
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add nsw i8 %x, 32
+  %c = icmp ult i8 %i, -32
+  ret i1 %c
+}
+
+define i1 @ult_add_nuw_nsw_C2_pow2_C_neg(i8 %x) {
+; CHECK-LABEL: @ult_add_nuw_nsw_C2_pow2_C_neg(
+; CHECK-NEXT:    [[C:%.*]] = icmp ult i8 [[X:%.*]], -64
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add nuw nsw i8 %x, 32
+  %c = icmp ult i8 %i, -32
+  ret i1 %c
+}
+
+define i1 @ult_add_C2_neg_C_pow2(i8 %x) {
+; CHECK-LABEL: @ult_add_C2_neg_C_pow2(
+; CHECK-NEXT:    [[TMP1:%.*]] = and i8 [[X:%.*]], -32
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i8 [[TMP1]], 32
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add i8 %x, -32
+  %c = icmp ult i8 %i, 32
+  ret i1 %c
+}
+
+define <2 x i1> @ult_add_C2_pow2_C_neg_vec(<2 x i8> %x) {
+; CHECK-LABEL: @ult_add_C2_pow2_C_neg_vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = and <2 x i8> [[X:%.*]], <i8 -32, i8 -32>
+; CHECK-NEXT:    [[C:%.*]] = icmp ne <2 x i8> [[TMP1]], <i8 -64, i8 -64>
+; CHECK-NEXT:    ret <2 x i1> [[C]]
+;
+  %i = add <2 x i8> %x, <i8 32, i8 32>
+  %c = icmp ult <2 x i8> %i, <i8 -32, i8 -32>
+  ret <2 x i1> %c
+}
+
+define i1 @uge_add_C2_pow2_C_neg(i8 %x) {
+; CHECK-LABEL: @uge_add_C2_pow2_C_neg(
+; CHECK-NEXT:    [[TMP1:%.*]] = and i8 [[X:%.*]], -32
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i8 [[TMP1]], -64
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add i8 %x, 32
+  %c = icmp uge i8 %i, -32
+  ret i1 %c
+}
+
 declare void @llvm.assume(i1)

@antoniofrighetto antoniofrighetto force-pushed the feature/instcombine-icmp-add branch from 2ed0210 to f4ff5cc Compare June 15, 2024 08:50
@goldsteinn
Copy link
Contributor

NB regarding the proofs, you can just do %C2_cnt = call i8 @llvm.ctpop.i8(i8 %C2); %C2_is_pow2 = icmp eq i8 %C2_cnt, 1 for power of 2 lemma.

@goldsteinn
Copy link
Contributor

LGTM, but can you look into CI failures.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key question here is whether this is the right canonicalization direction -- do we prefer a mask check over a range check? I guess if @dtcxzyw's tests don't show regressions from this, then this direction is fine.

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jun 15, 2024
@goldsteinn
Copy link
Contributor

The key question here is whether this is the right canonicalization direction -- do we prefer a mask check over a range check? I guess if @dtcxzyw's tests don't show regressions from this, then this direction is fine.

W.o nuw/nsw would def think mask check is better. W/ the flags could imagine the other way around though.

@dtcxzyw
Copy link
Member

dtcxzyw commented Jun 15, 2024

The key question here is whether this is the right canonicalization direction -- do we prefer a mask check over a range check? I guess if @dtcxzyw's tests don't show regressions from this, then this direction is fine.

Done.

@antoniofrighetto
Copy link
Contributor Author

Added a multi-use, thanks. CI failures look unrelated.

@antoniofrighetto antoniofrighetto force-pushed the feature/instcombine-icmp-add branch from 797932a to 21ac510 Compare June 16, 2024 09:30
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

`icmp ult (add X, C2), C` can be folded to `icmp ne (and X, C), 2C`,
subject to `C == -C2` and C2 being a power of 2.

Proofs: https://alive2.llvm.org/ce/z/P-VVmQ.

Fixes: llvm#75613.
@antoniofrighetto antoniofrighetto force-pushed the feature/instcombine-icmp-add branch from 21ac510 to a4b44c0 Compare June 17, 2024 17:34
@antoniofrighetto antoniofrighetto merged commit a4b44c0 into llvm:main Jun 17, 2024
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Value Range: clang cannot know (a >> 3 == -2) means a in [ -16, -9]
5 participants