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

[ConstraintElim] Use SCEV to check for multiples #76925

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jan 4, 2024

When adding constraints for induction variables, if the step is not one, we need to make sure that (end-start) is a multiple of step, otherwise we might step over the end value.

Currently this only supports one specific pattern for pointers, where the end is a gep of the start with an appropriate offset.

Generalize this by using SCEV to check for multiples, which also makes this work for integer IVs.

There is no impact on compile-time.

When adding constraints for induction variables, if the step is
not one, we need to make sure that (end-start) is a multiple of
step, otherwise we might step over the end value.

Currently this only supports one specific pattern for pointers,
where the end is a gep of the start with an appropriate offset.

Generalize this by using SCEV to check for multiples, which also
makes this work for integer IVs.

There is no impact on compile-time.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

When adding constraints for induction variables, if the step is not one, we need to make sure that (end-start) is a multiple of step, otherwise we might step over the end value.

Currently this only supports one specific pattern for pointers, where the end is a gep of the start with an appropriate offset.

Generalize this by using SCEV to check for multiples, which also makes this work for integer IVs.

There is no impact on compile-time.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+4-17)
  • (modified) llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-multiples.ll (+5-10)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index cc93c8617c05e2..5e57aa78175164 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -968,23 +968,10 @@ void State::addInfoForInductions(BasicBlock &BB) {
     return;
 
   if (!StepOffset.isOne()) {
-    auto *UpperGEP = dyn_cast<GetElementPtrInst>(B);
-    if (!UpperGEP || UpperGEP->getPointerOperand() != StartValue ||
-        !UpperGEP->isInBounds())
-      return;
-
-    MapVector<Value *, APInt> UpperVariableOffsets;
-    APInt UpperConstantOffset(StepOffset.getBitWidth(), 0);
-    const DataLayout &DL = BB.getModule()->getDataLayout();
-    if (!UpperGEP->collectOffset(DL, StepOffset.getBitWidth(),
-                                 UpperVariableOffsets, UpperConstantOffset))
-      return;
-    // All variable offsets and the constant offset have to be a multiple of the
-    // step.
-    if (!UpperConstantOffset.urem(StepOffset).isZero() ||
-        any_of(UpperVariableOffsets, [&StepOffset](const auto &P) {
-          return !P.second.urem(StepOffset).isZero();
-        }))
+    // Check whether B-Start is known to be a multiple of StepOffset.
+    const SCEV *BMinusStart = SE.getMinusSCEV(SE.getSCEV(B), StartSCEV);
+    if (isa<SCEVCouldNotCompute>(BMinusStart) ||
+        !SE.getConstantMultiple(BMinusStart).urem(StepOffset).isZero())
       return;
   }
 
diff --git a/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-multiples.ll b/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-multiples.ll
index 035cea4d73246c..a952c4048b450a 100644
--- a/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-multiples.ll
+++ b/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-multiples.ll
@@ -13,8 +13,7 @@ define void @multiple_pow2(i64 %count) {
 ; CHECK-NEXT:    [[CMP_I_NOT:%.*]] = icmp eq i64 [[IV]], [[END]]
 ; CHECK-NEXT:    br i1 [[CMP_I_NOT]], label [[EXIT:%.*]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[CMP2_I_I:%.*]] = icmp ult i64 [[IV]], [[END]]
-; CHECK-NEXT:    br i1 [[CMP2_I_I]], label [[LOOP]], label [[EXIT]]
+; CHECK-NEXT:    br i1 true, label [[LOOP]], label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
@@ -48,8 +47,7 @@ define void @multiple_pow2_larger_than_needed(i64 %count) {
 ; CHECK-NEXT:    [[CMP_I_NOT:%.*]] = icmp eq i64 [[IV]], [[END]]
 ; CHECK-NEXT:    br i1 [[CMP_I_NOT]], label [[EXIT:%.*]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[CMP2_I_I:%.*]] = icmp ult i64 [[IV]], [[END]]
-; CHECK-NEXT:    br i1 [[CMP2_I_I]], label [[LOOP]], label [[EXIT]]
+; CHECK-NEXT:    br i1 true, label [[LOOP]], label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
@@ -119,8 +117,7 @@ define void @multiple_pow2_start_offset(i64 %count) {
 ; CHECK-NEXT:    [[CMP_I_NOT:%.*]] = icmp eq i64 [[IV]], [[END]]
 ; CHECK-NEXT:    br i1 [[CMP_I_NOT]], label [[EXIT]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[CMP2_I_I:%.*]] = icmp ult i64 [[IV]], [[END]]
-; CHECK-NEXT:    br i1 [[CMP2_I_I]], label [[LOOP]], label [[EXIT]]
+; CHECK-NEXT:    br i1 true, label [[LOOP]], label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
@@ -194,8 +191,7 @@ define void @multiple_pow2_start_offset_dynamic(i64 %count) {
 ; CHECK-NEXT:    [[CMP_I_NOT:%.*]] = icmp eq i64 [[IV]], [[END]]
 ; CHECK-NEXT:    br i1 [[CMP_I_NOT]], label [[EXIT]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[CMP2_I_I:%.*]] = icmp ult i64 [[IV]], [[END]]
-; CHECK-NEXT:    br i1 [[CMP2_I_I]], label [[LOOP]], label [[EXIT]]
+; CHECK-NEXT:    br i1 true, label [[LOOP]], label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
@@ -231,8 +227,7 @@ define void @multiple_non_pow2_nuw(i64 %count) {
 ; CHECK-NEXT:    [[CMP_I_NOT:%.*]] = icmp eq i64 [[IV]], [[END]]
 ; CHECK-NEXT:    br i1 [[CMP_I_NOT]], label [[EXIT:%.*]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[CMP2_I_I:%.*]] = icmp ult i64 [[IV]], [[END]]
-; CHECK-NEXT:    br i1 [[CMP2_I_I]], label [[LOOP]], label [[EXIT]]
+; CHECK-NEXT:    br i1 true, label [[LOOP]], label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jan 4, 2024
dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jan 4, 2024
Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nikic nikic merged commit f812251 into llvm:main Jan 4, 2024
5 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.

None yet

3 participants