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 isKnownNonNegative for condition transfer. #72879

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Nov 20, 2023

Use isKnownNonNegative for information transfer. This can improve results, in cases where ValueTracking can infer additional non-negative info, e.g. for phi nodes.

This allows simplifying the check from
#63126 by ConstraintElimination. It is also simplified by IndVarSimplify now; note the changes in llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll, due to this now being simplified earlier.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 20, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Use isKnownNonNegative for SLT information transfer. This can improve results, in cases where ValueTracking can infer additional non-negative info, e.g. for phi nodes.

This allows simplifying the check from
#63126 by ConstraintElimination. It is also simplifyed by IndVarSimplify now; note the changes in llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll, due to this now being simplified earlier.


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+2-1)
  • (modified) llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll (+3-6)
  • (modified) llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll (+4-4)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 8ec2ee47db1b70c..350b561ca53deaf 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -804,7 +804,8 @@ void ConstraintInfo::transferToOtherSystem(
     }
     break;
   case CmpInst::ICMP_SLT:
-    if (doesHold(CmpInst::ICMP_SGE, A, ConstantInt::get(B->getType(), 0)))
+    if (doesHold(CmpInst::ICMP_SGE, A, ConstantInt::get(B->getType(), 0)) ||
+        isKnownNonNegative(A, DL, /*Depth=*/MaxAnalysisRecursionDepth - 1))
       addFact(CmpInst::ICMP_ULT, A, B, NumIn, NumOut, DFSInStack);
     break;
   case CmpInst::ICMP_SGT: {
diff --git a/llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll b/llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll
index 41596dbf3e3365e..46cc306466f182c 100644
--- a/llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll
+++ b/llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll
@@ -750,8 +750,7 @@ define i8 @iv_known_non_negative_constant_trip_count(ptr %dst, i8 %N) {
 ; CHECK-NEXT:    call void @use(i1 [[F_1]])
 ; CHECK-NEXT:    [[F_2:%.*]] = icmp sle i8 [[N]], [[IV]]
 ; CHECK-NEXT:    call void @use(i1 [[F_2]])
-; CHECK-NEXT:    [[C_0:%.*]] = icmp ugt i8 [[IV]], 2
-; CHECK-NEXT:    call void @use(i1 [[C_0]])
+; CHECK-NEXT:    call void @use(i1 false)
 ; CHECK-NEXT:    [[IV_NEXT]] = add nuw nsw i8 [[IV]], 1
 ; CHECK-NEXT:    br label [[LOOP_HEADER]]
 ; CHECK:       exit.1:
@@ -842,11 +841,9 @@ define i8 @iv_known_non_negative_variable_trip_count(ptr %dst, i8 %N) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8 [[IV]], [[N:%.*]]
 ; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_LATCH]], label [[EXIT_1:%.*]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[T_1:%.*]] = icmp ugt i8 [[N]], [[IV]]
-; CHECK-NEXT:    call void @use(i1 [[T_1]])
 ; CHECK-NEXT:    call void @use(i1 true)
-; CHECK-NEXT:    [[F_1:%.*]] = icmp ule i8 [[N]], [[IV]]
-; CHECK-NEXT:    call void @use(i1 [[F_1]])
+; CHECK-NEXT:    call void @use(i1 true)
+; CHECK-NEXT:    call void @use(i1 false)
 ; CHECK-NEXT:    call void @use(i1 false)
 ; CHECK-NEXT:    [[C_0:%.*]] = icmp ugt i8 [[IV]], 2
 ; CHECK-NEXT:    call void @use(i1 [[C_0]])
diff --git a/llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll b/llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll
index e1eac5f80485494..dbe29a071092e40 100644
--- a/llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll
+++ b/llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll
@@ -280,18 +280,18 @@ define void @loop_with_signed_induction(ptr noundef nonnull align 8 dereferencea
 ; CHECK-NEXT:    [[SUB_PTR_SUB_I_I:%.*]] = sub i64 [[SUB_PTR_LHS_CAST_I_I]], [[SUB_PTR_RHS_CAST_I_I]]
 ; CHECK-NEXT:    [[SUB_PTR_DIV_I_I:%.*]] = ashr exact i64 [[SUB_PTR_SUB_I_I]], 3
 ; CHECK-NEXT:    [[CMP9:%.*]] = icmp sgt i64 [[SUB_PTR_DIV_I_I]], 0
-; CHECK-NEXT:    br i1 [[CMP9]], label [[OPERATOR_ACC_EXIT:%.*]], label [[FOR_COND_CLEANUP:%.*]]
+; CHECK-NEXT:    br i1 [[CMP9]], label [[FOR_BODY:%.*]], label [[FOR_COND_CLEANUP:%.*]]
 ; CHECK:       for.cond.cleanup:
 ; CHECK-NEXT:    ret void
-; CHECK:       operator_acc.exit:
-; CHECK-NEXT:    [[I_010:%.*]] = phi i64 [ [[INC:%.*]], [[OPERATOR_ACC_EXIT]] ], [ 0, [[ENTRY:%.*]] ]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[I_010:%.*]] = phi i64 [ [[INC:%.*]], [[FOR_BODY]] ], [ 0, [[ENTRY:%.*]] ]
 ; CHECK-NEXT:    [[ADD_PTR_I:%.*]] = getelementptr inbounds double, ptr [[TMP1]], i64 [[I_010]]
 ; CHECK-NEXT:    [[TMP2:%.*]] = load double, ptr [[ADD_PTR_I]], align 8, !tbaa [[TBAA6:![0-9]+]]
 ; CHECK-NEXT:    [[ADD:%.*]] = fadd double [[TMP2]], 1.000000e+00
 ; CHECK-NEXT:    store double [[ADD]], ptr [[ADD_PTR_I]], align 8, !tbaa [[TBAA6]]
 ; CHECK-NEXT:    [[INC]] = add nuw nsw i64 [[I_010]], 1
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[INC]], [[SUB_PTR_DIV_I_I]]
-; CHECK-NEXT:    br i1 [[CMP]], label [[OPERATOR_ACC_EXIT]], label [[FOR_COND_CLEANUP]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[FOR_BODY]], label [[FOR_COND_CLEANUP]]
 ;
 entry:
   %vec.addr = alloca ptr, align 8

fhahn added a commit that referenced this pull request Nov 20, 2023
Extend test coverage to include all predicates we support transfers for.
@fhahn fhahn force-pushed the ce-isknownnonnegative branch 2 times, most recently from e5c30cd to 94e7214 Compare November 20, 2023 22:47
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

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@antoniofrighetto antoniofrighetto 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! (Perhaps commit description can be generalized not only to SLT now, simplifyed -> simplified).

Use isKnownNonNegative for SLT information transfer. This can improve
results, in cases where ValueTracking can infer additional non-negative
info, e.g. for phi nodes.

This allows simplifying the check from
llvm#63126 by
ConstraintElimination. It is also simplifyed by IndVarSimplify now; note
the changes in llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll,
due to this now being simplified earlier.
@fhahn
Copy link
Contributor Author

fhahn commented Nov 21, 2023

LGTM, thanks! (Perhaps commit description can be generalized not only to SLT now, simplifyed -> simplified).

Updated the message (I only updated the commit message, not the PR description.....)

@fhahn fhahn changed the title [ConstraintElim] Use isKnownNonNegative for signed->unsigned transfer. [ConstraintElim] Use isKnownNonNegative for condition transfer. Nov 21, 2023
@fhahn fhahn merged commit 4ccdab3 into llvm:main Nov 21, 2023
2 of 3 checks passed
@fhahn fhahn deleted the ce-isknownnonnegative branch November 21, 2023 10:09
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.

4 participants