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] Fold the bound check idiom into sign bit test #76439

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 27, 2023

This patch folds the bound check idiom:

T *end, *start;
(size_t)(end - start) > (size_t)(PTRDIFF_MAX / sizeof(T))

into

(ptrdiff_t)(end - start) < 0

i.e.:

icmp ugt (sdiv exact X, C2), (sdiv signed_max, C2) --> icmp slt X, 0
icmp ult (sdiv exact X, C2), (sdiv signed_max, C2) + 1 --> icmp sgt X, -1

Alive2: https://alive2.llvm.org/ce/z/feXjoj

NOTE: It also holds for ashr exact when sizeof(T) is a power of 2.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 27, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch folds the bound check idiom:

T *end, *start;
(size_t)(end - start) &gt; (size_t)(PTRDIFF_MAX / sizeof(T))

into

(ptrdiff_t)(end - start) &lt; 0

i.e.:

icmp ugt (sdiv exact X, C2), (sdiv signed_max, C2) --&gt; icmp slt X, 0
icmp ult (sdiv exact X, C2), (sdiv signed_max, C2) + 1 --&gt; icmp sgt X, -1

Alive2: https://alive2.llvm.org/ce/z/feXjoj

NOTE: It also holds for ashr exact when sizeof(T) is a power of 2.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+20)
  • (modified) llvm/test/Transforms/InstCombine/icmp.ll (+95)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 0222c93faf24e9..43d1562a3177d4 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2636,6 +2636,26 @@ Instruction *InstCombinerImpl::foldICmpDivConstant(ICmpInst &Cmp,
   if (!match(Y, m_APInt(C2)))
     return nullptr;
 
+  // Fold the bound check idiom:
+  //  T *end, *start;
+  //  (size_t)(end - start) > (size_t)(PTRDIFF_MAX / sizeof(T))
+  // into:
+  //  (ptrdiff_t)(end - start) < 0
+  // i.e.:
+  //   icmp ugt (sdiv exact X, C2), (sdiv signed_max, C2) --> icmp slt X, 0
+  //   icmp ult (sdiv exact X, C2), (sdiv signed_max, C2) + 1 --> icmp sgt X, -1
+  // where C2 is positive.
+  if (DivIsSigned && Div->isExact() &&
+      (Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_ULT) &&
+      C2->isStrictlyPositive() &&
+      APInt::getSignedMaxValue(C2->getBitWidth()).sdiv(*C2) +
+              APInt(C2->getBitWidth(), Pred == ICmpInst::ICMP_UGT ? 0 : 1) ==
+          C)
+    return new ICmpInst(
+        Pred == ICmpInst::ICMP_UGT ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_SGT, X,
+        Pred == ICmpInst::ICMP_UGT ? Constant::getNullValue(X->getType())
+                                   : Constant::getAllOnesValue(X->getType()));
+
   // FIXME: If the operand types don't match the type of the divide
   // then don't attempt this transform. The code below doesn't have the
   // logic to deal with a signed divide and an unsigned compare (and
diff --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll
index 9b2e141bdb0506..48a33b65177714 100644
--- a/llvm/test/Transforms/InstCombine/icmp.ll
+++ b/llvm/test/Transforms/InstCombine/icmp.ll
@@ -5013,3 +5013,98 @@ define i1 @or_positive_sgt_zero_multi_use(i8 %a) {
   %cmp = icmp sgt i8 %b, 0
   ret i1 %cmp
 }
+
+define i1 @icmp_ugt_sdiv_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_sdiv_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[X:%.*]], 0
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv exact i64 %x, 24
+  %cmp = icmp ugt i64 %sdiv, 384307168202282325
+  ret i1 %cmp
+}
+
+define i1 @icmp_ult_sdiv_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ult_sdiv_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[X:%.*]], -1
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv exact i64 %x, 24
+  %cmp = icmp ult i64 %sdiv, 384307168202282326
+  ret i1 %cmp
+}
+
+; TODO: This should be simplified to icmp slt i64 %x, 0
+define i1 @icmp_ugt_ashr_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_ashr_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i64 [[X:%.*]], 9223372036854775804
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = ashr exact i64 %x, 2
+  %cmp = icmp ugt i64 %sdiv, 2305843009213693951
+  ret i1 %cmp
+}
+
+define i1 @icmp_ult_ashr_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ult_ashr_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[X:%.*]], -1
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = ashr exact i64 %x, 2
+  %cmp = icmp ult i64 %sdiv, 2305843009213693952
+  ret i1 %cmp
+}
+
+; Negative tests
+define i1 @icmp_ugt_sdiv_by_constant_without_exact(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_sdiv_by_constant_without_exact(
+; CHECK-NEXT:    [[SDIV:%.*]] = sdiv i64 [[X:%.*]], 24
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i64 [[SDIV]], 384307168202282325
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv i64 %x, 24
+  %cmp = icmp ugt i64 %sdiv, 384307168202282325
+  ret i1 %cmp
+}
+
+define i1 @icmp_ugt_udiv_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_udiv_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i64 [[X:%.*]], 9223372036854775800
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = udiv exact i64 %x, 24
+  %cmp = icmp ugt i64 %sdiv, 384307168202282325
+  ret i1 %cmp
+}
+
+define i1 @icmp_ne_sdiv_by_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ne_sdiv_by_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i64 [[X:%.*]], 9223372036854775800
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv exact i64 %x, 24
+  %cmp = icmp ne i64 %sdiv, 384307168202282325
+  ret i1 %cmp
+}
+
+define i1 @icmp_ugt_sdiv_by_constant_wrong_rhs(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_sdiv_by_constant_wrong_rhs(
+; CHECK-NEXT:    [[SDIV:%.*]] = sdiv exact i64 [[X:%.*]], 24
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i64 [[SDIV]], 384307168202282324
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv exact i64 %x, 24
+  %cmp = icmp ugt i64 %sdiv, 384307168202282324
+  ret i1 %cmp
+}
+
+define i1 @icmp_ugt_sdiv_by_negative_constant(i64 %x) {
+; CHECK-LABEL: @icmp_ugt_sdiv_by_negative_constant(
+; CHECK-NEXT:    [[SDIV:%.*]] = sdiv i64 [[X:%.*]], -24
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i64 [[SDIV]], -384307168202282326
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %sdiv = sdiv i64 %x, -24
+  %cmp = icmp ugt i64 %sdiv, -384307168202282326
+  ret i1 %cmp
+}

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Dec 27, 2023
dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Dec 28, 2023
C2->isStrictlyPositive() &&
APInt::getSignedMaxValue(C2->getBitWidth()).sdiv(*C2) +
APInt(C2->getBitWidth(), Pred == ICmpInst::ICMP_UGT ? 0 : 1) ==
C)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the special case to signed max? Doesn't this hold pretty generally? https://alive2.llvm.org/ce/z/zv2kqx (The ugt would become slt 0 for the right boundary.)

Copy link
Member Author

Choose a reason for hiding this comment

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

The ugt would become slt 0 for the right boundary.

We will fold icmp ugt i64 (sdiv exact i64 %x, 24), 384307168202282325 into icmp ugt i64 %x, 9223372036854775800. The latter form cannot be simplified into icmp slt i64 %x, 0.
Alive2: https://alive2.llvm.org/ce/z/WEPmRN

Copy link
Contributor

Choose a reason for hiding this comment

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

As the division is exact, there are multiple legal constants that can be used (we can add up to C2-1 assuming it does not overflow). We could special case this to produce a better constant for the sign boundary. Or just leave the ugt, as this is still much better than the division, while covering more cases.

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.

None yet

3 participants