[SCEV] Don't require NUW at first add when checking A+C1 < (A+C2)<nuw> #149795

fhahn · 2025-07-21T10:52:34Z

Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2.

https://alive2.llvm.org/ce/z/b3dM7N

llvmbot · 2025-07-21T10:53:08Z

@llvm/pr-subscribers-llvm-analysis

Author: Florian Hahn (fhahn)

Changes

Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2.

https://alive2.llvm.org/ce/z/b3dM7N

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

3 Files Affected:

(modified) llvm/lib/Analysis/ScalarEvolution.cpp (+17-7)
(modified) llvm/test/Analysis/LoopAccessAnalysis/different-strides-safe-dep-due-to-backedge-taken-count.ll (+1-7)
(modified) llvm/test/Analysis/LoopAccessAnalysis/positive-dependence-distance-different-access-sizes.ll (+1-12)

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 24adfa346c642..b12ce6dbeb4d3 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -11418,8 +11418,7 @@ bool ScalarEvolution::isKnownPredicateViaNoOverflow(CmpPredicate Pred,
       XNonConstOp = X;
       XFlagsPresent = ExpectedFlags;
     }
-    if (!isa<SCEVConstant>(XConstOp) ||
-        (XFlagsPresent & ExpectedFlags) != ExpectedFlags)
+    if (!isa<SCEVConstant>(XConstOp))
       return false;
 
     if (!splitBinaryAdd(Y, YConstOp, YNonConstOp, YFlagsPresent)) {
@@ -11428,13 +11427,24 @@ bool ScalarEvolution::isKnownPredicateViaNoOverflow(CmpPredicate Pred,
       YFlagsPresent = ExpectedFlags;
     }
 
-    if (!isa<SCEVConstant>(YConstOp) ||
-        (YFlagsPresent & ExpectedFlags) != ExpectedFlags)
+    if (YNonConstOp != XNonConstOp)
       return false;
 
-    if (YNonConstOp != XNonConstOp)
+    if (!isa<SCEVConstant>(YConstOp))
       return false;
 
+    // When matching ADDs with NUW flags (and unsigned predicates), only the
+    // second ADD (with the larger constant) requires NUW.
+    if (YNonConstOp != Y && ExpectedFlags == SCEV::FlagNUW) {
+      if ((YFlagsPresent & ExpectedFlags) != ExpectedFlags)
+        return false;
+    } else {
+      if ((XFlagsPresent & ExpectedFlags) != ExpectedFlags)
+        return false;
+      if ((YFlagsPresent & ExpectedFlags) != ExpectedFlags)
+        return false;
+    }
+
     OutC1 = cast<SCEVConstant>(XConstOp)->getAPInt();
     OutC2 = cast<SCEVConstant>(YConstOp)->getAPInt();
 
@@ -11472,7 +11482,7 @@ bool ScalarEvolution::isKnownPredicateViaNoOverflow(CmpPredicate Pred,
     std::swap(LHS, RHS);
     [[fallthrough]];
   case ICmpInst::ICMP_ULE:
-    // (X + C1)<nuw> u<= (X + C2)<nuw> for C1 u<= C2.
+    // (X + C1) u<= (X + C2)<nuw> for C1 u<= C2.
     if (MatchBinaryAddToConst(LHS, RHS, C1, C2, SCEV::FlagNUW) && C1.ule(C2))
       return true;
 
@@ -11482,7 +11492,7 @@ bool ScalarEvolution::isKnownPredicateViaNoOverflow(CmpPredicate Pred,
     std::swap(LHS, RHS);
     [[fallthrough]];
   case ICmpInst::ICMP_ULT:
-    // (X + C1)<nuw> u< (X + C2)<nuw> if C1 u< C2.
+    // (X + C1) u< (X + C2)<nuw> if C1 u< C2.
     if (MatchBinaryAddToConst(LHS, RHS, C1, C2, SCEV::FlagNUW) && C1.ult(C2))
       return true;
     break;
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/different-strides-safe-dep-due-to-backedge-taken-count.ll b/llvm/test/Analysis/LoopAccessAnalysis/different-strides-safe-dep-due-to-backedge-taken-count.ll
index 72b620aad31dc..311de84993001 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/different-strides-safe-dep-due-to-backedge-taken-count.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/different-strides-safe-dep-due-to-backedge-taken-count.ll
@@ -106,17 +106,11 @@ exit:
   ret void
 }
 
-; TOOD: The loop should be safe without dependence, as all accesses to %l are
-; completely before the first store.
 define void @backward_dep_known_safe_due_to_backedge_taken_count(ptr %A) {
 ; CHECK-LABEL: 'backward_dep_known_safe_due_to_backedge_taken_count'
 ; CHECK-NEXT:    loop:
-; CHECK-NEXT:      Memory dependences are safe with a maximum safe vector width of 8160 bits
+; CHECK-NEXT:      Memory dependences are safe
 ; CHECK-NEXT:      Dependences:
-; CHECK-NEXT:        BackwardVectorizable:
-; CHECK-NEXT:            %l = load i32, ptr %gep, align 4 ->
-; CHECK-NEXT:            store i32 %add, ptr %gep.mul.2, align 4
-; CHECK-EMPTY:
 ; CHECK-NEXT:      Run-time memory checks:
 ; CHECK-NEXT:      Grouped accesses:
 ; CHECK-EMPTY:
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/positive-dependence-distance-different-access-sizes.ll b/llvm/test/Analysis/LoopAccessAnalysis/positive-dependence-distance-different-access-sizes.ll
index 1a6e25859f085..468b22568277e 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/positive-dependence-distance-different-access-sizes.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/positive-dependence-distance-different-access-sizes.ll
@@ -8,21 +8,10 @@ target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
 define void @test_distance_positive_independent_via_trip_count(ptr %A) {
 ; CHECK-LABEL: 'test_distance_positive_independent_via_trip_count'
 ; CHECK-NEXT:    loop:
-; CHECK-NEXT:      Memory dependences are safe with run-time checks
+; CHECK-NEXT:      Memory dependences are safe
 ; CHECK-NEXT:      Dependences:
 ; CHECK-NEXT:      Run-time memory checks:
-; CHECK-NEXT:      Check 0:
-; CHECK-NEXT:        Comparing group GRP0:
-; CHECK-NEXT:          %gep.A.400 = getelementptr inbounds i32, ptr %A.400, i64 %iv
-; CHECK-NEXT:        Against group GRP1:
-; CHECK-NEXT:          %gep.A = getelementptr inbounds i8, ptr %A, i64 %iv
 ; CHECK-NEXT:      Grouped accesses:
-; CHECK-NEXT:        Group GRP0:
-; CHECK-NEXT:          (Low: (400 + %A)<nuw> High: (804 + %A))
-; CHECK-NEXT:            Member: {(400 + %A)<nuw>,+,4}<nuw><%loop>
-; CHECK-NEXT:        Group GRP1:
-; CHECK-NEXT:          (Low: %A High: (101 + %A))
-; CHECK-NEXT:            Member: {%A,+,1}<nuw><%loop>
 ; CHECK-EMPTY:
 ; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
 ; CHECK-NEXT:      SCEV assumptions:

nikic · 2025-07-22T14:38:12Z

llvm/lib/Analysis/ScalarEvolution.cpp

Why is this YNonConstOp != Y check necessary? If they're the same (and YConstOp zero) then the flags are initialized to match and the check should pass anywa?

Yep, removed the check, thanks

nikic · 2025-07-22T14:38:34Z

llvm/lib/Analysis/ScalarEvolution.cpp

Avoid repeating the YFlagsPresent check in both branches?

Yup, hoisted out thanks.

Looks like there's still a leftover check here.

Argh yes, pushed the missing changes.

nikic

Technically we could propagate the nuw flag from A+C2 to A+C1, but I guess we don't really have a good place to actually do that.

Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2. https://alive2.llvm.org/ce/z/b3dM7N

fhahn

Technically we could propagate the nuw flag from A+C2 to A+C1, but I guess we don't really have a good place to actually do that.

Yep that's a bit unforuntate, maybe there's a better general solution than just trying to re-write flags ad-hoc when construction other SCEVs.

fhahn · 2025-07-22T18:29:41Z

llvm/lib/Analysis/ScalarEvolution.cpp

Yup, hoisted out thanks.

fhahn · 2025-07-22T18:30:01Z

llvm/lib/Analysis/ScalarEvolution.cpp

Yep, removed the check, thanks

nikic

LGTM

… (A+C2)<nuw> (#149795) Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2. https://alive2.llvm.org/ce/z/b3dM7N PR: llvm/llvm-project#149795

llvm#149795) Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2. https://alive2.llvm.org/ce/z/b3dM7N PR: llvm#149795

llvm#149795) Relax the NUW requirements for isKnownPredicateViaNoOverflow, if the second operand (Y) is an ADD. The code only simplifies the condition if C1 < C2, so if the second ADD is NUW, it doesn't matter whether the first operand also has the NUW flag, as it cannot wrap if C1 < C2. https://alive2.llvm.org/ce/z/b3dM7N PR: llvm#149795 (cherry picked from commit 6c50e2b)

fhahn requested review from efriedma-quic and preames July 21, 2025 10:52

fhahn requested a review from nikic as a code owner July 21, 2025 10:52

llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Jul 21, 2025

nikic reviewed Jul 22, 2025

View reviewed changes

fhahn added 2 commits July 22, 2025 19:19

!fixup simplify checks as suggested, thanks

85fd68e

fhahn force-pushed the scev-isKnownPredicateViaNoOverflow-nuw-pos branch from 4d289f7 to 85fd68e Compare July 22, 2025 18:29

fhahn commented Jul 22, 2025

View reviewed changes

!fixup strip more

1a71b35

nikic approved these changes Jul 22, 2025

View reviewed changes

fhahn merged commit 6c50e2b into llvm:main Jul 23, 2025
9 checks passed

fhahn deleted the scev-isKnownPredicateViaNoOverflow-nuw-pos branch July 23, 2025 08:33

[SCEV] Don't require NUW at first add when checking A+C1 < (A+C2)<nuw> #149795

[SCEV] Don't require NUW at first add when checking A+C1 < (A+C2)<nuw> #149795

Uh oh!

Conversation

fhahn commented Jul 21, 2025

Uh oh!

llvmbot commented Jul 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants