[DA] Fix symbolic RDIV and Strong SIV with impossible assumptions (PR149501)

Address GitHub issue #149501.
Fixes incorrect independence conclusions in symbolic RDIV test and improves
Strong SIV bound checking for symbolic expressions.

The patch fixes:

- Strong SIV: Improve bound checking for symbolic expressions, avoid impossible
  assumptions like '0 <= -1' when Delta and UpperBound have inverse relationships.

- Symbolic RDIV: Detect when C2-C1 == N1+1 creates trivially true comparisons
  leading to wrong 'none!' conclusions, now returns conservative analysis. The
  symbolic RDIV fix prevents cases where expressions like (m-n) > (m-n-1)
  incorrectly conclude independence. Now returns conservative 'output [*|<]!'
  instead of wrong 'none!' for symbolic expressions with problematic bounds.

Author: sebpop

llvm/lib/Analysis/DependenceAnalysis.cpp

@@ -1243,35 +1243,72 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
   if (const SCEV *UpperBound = collectUpperBound(CurLoop, Delta->getType())) {
     LLVM_DEBUG(dbgs() << "\t    UpperBound = " << *UpperBound);
     LLVM_DEBUG(dbgs() << ", " << *UpperBound->getType() << "\n");
+
+    // Handle negative upper bound - loop doesn't execute.
+    if (SE->isKnownNegative(UpperBound)) {
+      LLVM_DEBUG(dbgs() << "\t    Negative upper bound - no dependence\n");
+      ++StrongSIVindependence;
+      ++StrongSIVsuccesses;
+      return true;
+    }
+
     const SCEV *AbsDelta =
         SE->isKnownNonNegative(Delta) ? Delta : SE->getNegativeSCEV(Delta);
     const SCEV *AbsCoeff =
         SE->isKnownNonNegative(Coeff) ? Coeff : SE->getNegativeSCEV(Coeff);
     const SCEV *Product = SE->getMulExpr(UpperBound, AbsCoeff);
+
     if (isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product)) {
       // Check if this involves symbolic expressions where we might be too
       // conservative.
       if (isa<SCEVUnknown>(Delta) || isa<SCEVUnknown>(Coeff) ||
           !isa<SCEVConstant>(AbsDelta) || !isa<SCEVConstant>(Product)) {
-        // For symbolic expressions, add runtime assumption rather than
-        // rejecting.
-        const SCEVPredicate *BoundPred =
-            SE->getComparePredicate(ICmpInst::ICMP_SLE, AbsDelta, Product);
-        if (UnderRuntimeAssumptions) {
-          SmallVector<const SCEVPredicate *, 4> NewPreds(
-              Assumptions.getPredicates());
-          NewPreds.push_back(BoundPred);
-          const_cast<DependenceInfo *>(this)->Assumptions =
-              SCEVUnionPredicate(NewPreds, *SE);
-          LLVM_DEBUG(dbgs() << "\t    Added runtime bound assumption\n");
+        // Check if the assumption would be meaningful and not trivially
+        // impossible. For relationships like Delta=n-m, UpperBound=m-n-1, the
+        // assumption |n-m| <= (m-n-1) can be impossible (e.g., 0 <= -1 when
+        // n=m).
+        bool MeaningfulAssumption = true;
+
+        // Detect cases where Delta and UpperBound have inverse relationships
+        // that could lead to impossible assumptions.
+        if (isa<SCEVAddExpr>(Delta) && isa<SCEVAddExpr>(UpperBound)) {
+          // Look for patterns where UpperBound = -Delta - 1 (or similar).
+          const SCEV *NegDelta = SE->getNegativeSCEV(Delta);
+          const SCEV *NegDeltaMinusOne =
+              SE->getAddExpr(NegDelta, SE->getConstant(Delta->getType(), -1));
+          if (UpperBound == NegDeltaMinusOne) {
+            MeaningfulAssumption = false;
+            LLVM_DEBUG(dbgs() << "\t    Detected inverse relationship - "
+                                 "assumption would be impossible\n");
+          }
+        }
+
+        if (MeaningfulAssumption && !SE->isKnownNonPositive(Product)) {
+          // For symbolic expressions, add runtime assumption rather than
+          // rejecting.
+          const SCEVPredicate *BoundPred =
+              SE->getComparePredicate(ICmpInst::ICMP_SLE, AbsDelta, Product);
+          if (UnderRuntimeAssumptions) {
+            SmallVector<const SCEVPredicate *, 4> NewPreds(
+                Assumptions.getPredicates());
+            NewPreds.push_back(BoundPred);
+            const_cast<DependenceInfo *>(this)->Assumptions =
+                SCEVUnionPredicate(NewPreds, *SE);
+            LLVM_DEBUG(dbgs() << "\t    Added runtime bound assumption\n");
+          } else {
+            // Cannot add runtime assumptions, let more complex tests try.
+            LLVM_DEBUG(dbgs() << "\t    Would need runtime bound assumption "
+                                 "but not allowed. Failing this test.\n");
+            return false;
+          }
         } else {
-          // Cannot add runtime assumptions, let more complex tests try.
-          LLVM_DEBUG(dbgs() << "\t    Would need runtime bound assumption but "
-                               "not allowed. Failing this test.\n");
+          LLVM_DEBUG(dbgs() << "\t    Cannot add meaningful assumption\n");
+          // When bound check fails and we can't add meaningful assumptions,
+          // this test cannot handle this case reliably.
           return false;
         }
       } else {
-        // Distance definitely greater than trip count - no dependence
+        // Distance definitely greater than trip count - no dependence.
         ++StrongSIVindependence;
         ++StrongSIVsuccesses;
         return true;
@@ -2189,8 +2226,39 @@ bool DependenceInfo::symbolicRDIVtest(const SCEV *A1, const SCEV *A2,
   const SCEV *N2 = collectUpperBound(Loop2, A1->getType());
   LLVM_DEBUG(if (N1) dbgs() << "\t    N1 = " << *N1 << "\n");
   LLVM_DEBUG(if (N2) dbgs() << "\t    N2 = " << *N2 << "\n");
+
   const SCEV *C2_C1 = SE->getMinusSCEV(C2, C1);
   const SCEV *C1_C2 = SE->getMinusSCEV(C1, C2);
+
+  // Check for negative or problematic upper bounds
+  auto CheckUpperBound = [&](const SCEV *N, const SCEV *Delta,
+                             const char *Name) -> bool {
+    if (!N)
+      // No bound to check.
+      return true;
+
+    if (SE->isKnownNegative(N)) {
+      LLVM_DEBUG(dbgs() << "\t    " << Name
+                        << " is negative - analysis unreliable\n");
+      return false;
+    }
+
+    // Check for degenerate cases where upper bounds are too close to the delta
+    // values. This can happen when N is an expression like (Delta - 1), leading
+    // to trivially true comparisons like Delta > (Delta - 1).
+    const SCEV *NPlusOne = SE->getAddExpr(N, SE->getOne(N->getType()));
+    if (Delta == NPlusOne) {
+      LLVM_DEBUG(dbgs() << "\t    Degenerate case: Delta == " << Name
+                        << "+1 - analysis unreliable\n");
+      return false;
+    }
+    // Bound is OK.
+    return true;
+  };
+
+  if (!CheckUpperBound(N1, C2_C1, "N1") || !CheckUpperBound(N2, C1_C2, "N2"))
+    return false;
+
   LLVM_DEBUG(dbgs() << "\t    C2 - C1 = " << *C2_C1 << "\n");
   LLVM_DEBUG(dbgs() << "\t    C1 - C2 = " << *C1_C2 << "\n");
   if (SE->isKnownNonNegative(A1)) {

llvm/test/Analysis/DependenceAnalysis/PR149501.ll

@@ -0,0 +1,90 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+
+; Test case for GitHub issue #149501: DA bound check with symbolic expressions.
+; The issue occurs when symbolic upper bounds and deltas create impossible runtime assumptions.
+; Our fix improves the bound check logic to handle these cases more gracefully.
+
+; Case 1: Symbolic case that was problematic - improved bound checking.
+define void @f_symbolic(ptr %a, i64 %n, i64 %m) {
+; CHECK-LABEL: 'f_symbolic'
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 40, ptr %idx.0, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - consistent output [*|<]!
+; CHECK-NEXT:  Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+;
+entry:
+  %bound = sub i64 %m, %n
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+  %subscript.0 = add i64 %i, %n
+  %subscript.1 = add i64 %i, %m
+  %idx.0 = getelementptr i8, ptr %a, i64 %subscript.0
+  %idx.1 = getelementptr i8, ptr %a, i64 %subscript.1
+  store i8 40, ptr %idx.0
+  store i8 42, ptr %idx.1
+  %i.next = add i64 %i, 1
+  %cond.exit = icmp eq i64 %i.next, %bound
+  br i1 %cond.exit, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+; Case 2: Case with negative bound - correctly handled by improved bound check.
+define void @f_negative_bound(ptr %a) {
+; CHECK-LABEL: 'f_negative_bound'
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 40, ptr %idx.0, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+  %idx.0 = getelementptr i8, ptr %a, i64 %i
+  %idx.1 = getelementptr i8, ptr %a, i64 %i
+  store i8 40, ptr %idx.0
+  store i8 42, ptr %idx.1
+  %i.next = add i64 %i, 1
+  %cond.exit = icmp eq i64 %i.next, 0    ; bound = 0 (negative upper bound)
+  br i1 %cond.exit, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+; Case 3: Same location access with positive bound - should show output dependence.
+define void @f_same_location(ptr %a) {
+; CHECK-LABEL: 'f_same_location'
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 40, ptr %idx.0, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 40, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - consistent output [0|<]!
+; CHECK-NEXT:  Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+  %idx.0 = getelementptr i8, ptr %a, i64 %i      ; a[i]
+  %idx.1 = getelementptr i8, ptr %a, i64 %i      ; a[i] - same location!
+  store i8 40, ptr %idx.0
+  store i8 42, ptr %idx.1
+  %i.next = add i64 %i, 1
+  %cond.exit = icmp eq i64 %i.next, 10   ; positive bound
+  br i1 %cond.exit, label %exit, label %loop
+
+exit:
+  ret void
+}