[DA] Fix zero coefficient bug in Strong SIV test using runtime assumptions

The Strong SIV test was incorrectly concluding "no dependence" when the
coefficient is symbolic and the delta (difference between source and
destination) is zero.

When delta=0, the Strong SIV test divides delta/coeff to get the distance.
The bug occurs when coeff is an unknown symbolic value: if coeff=0 at runtime,
then 0/0 is undefined and all iterations access the same memory location,
creating a true dependence that was being missed.

Author: sebpop

llvm/include/llvm/Analysis/DependenceAnalysis.h

@@ -554,7 +554,7 @@ class DependenceInfo {
   /// If the dependence isn't proven to exist,
   /// marks the Result as inconsistent.
   bool testSIV(const SCEV *Src, const SCEV *Dst, unsigned &Level,
-               FullDependence &Result) const;
+               FullDependence &Result, bool UnderRuntimeAssumptions);
 
   /// testRDIV - Tests the RDIV subscript pair (Src and Dst) for dependence.
   /// Things of the form [c1 + a1*i] and [c2 + a2*j]
@@ -584,7 +584,7 @@ class DependenceInfo {
   bool strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
                      const SCEV *DstConst, const Loop *CurrentSrcLoop,
                      const Loop *CurrentDstLoop, unsigned Level,
-                     FullDependence &Result) const;
+                     FullDependence &Result, bool UnderRuntimeAssumptions);
 
   /// weakCrossingSIVtest - Tests the weak-crossing SIV subscript pair
   /// (Src and Dst) for dependence.

llvm/lib/Analysis/DependenceAnalysis.cpp

@@ -1373,7 +1373,8 @@ bool DependenceInfo::testZIV(const SCEV *Src, const SCEV *Dst,
 bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
                                    const SCEV *DstConst, const Loop *CurSrcLoop,
                                    const Loop *CurDstLoop, unsigned Level,
-                                   FullDependence &Result) const {
+                                   FullDependence &Result,
+                                   bool UnderRuntimeAssumptions) {
   if (!isDependenceTestEnabled(DependenceTestType::StrongSIV))
     return false;
 
@@ -1445,7 +1446,38 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
       Result.DV[Level].Direction &= Dependence::DVEntry::EQ;
     ++StrongSIVsuccesses;
   } else if (Delta->isZero()) {
-    // since 0/X == 0
+    // Check if coefficient could be zero. If so, 0/0 is undefined and we
+    // cannot conclude that only same-iteration dependencies exist.
+    // When coeff=0, all iterations access the same location.
+    if (isa<SCEVUnknown>(Coeff)) {
+      // Use both isKnownNonZero and range analysis to prove coefficient != 0.
+      bool CoeffKnownNonZero = SE->isKnownNonZero(Coeff) ||
+                               isKnownPredicate(CmpInst::ICMP_NE, Coeff,
+                                                SE->getZero(Coeff->getType()));
+      if (!CoeffKnownNonZero) {
+        // Cannot prove at compile time, would need runtime assumption.
+        if (UnderRuntimeAssumptions) {
+          const SCEVPredicate *Pred = SE->getComparePredicate(
+              ICmpInst::ICMP_NE, Coeff, SE->getZero(Coeff->getType()));
+          SmallVector<const SCEVPredicate *, 4> NewPreds(
+              Result.Assumptions.getPredicates());
+          NewPreds.push_back(Pred);
+          Result.Assumptions = SCEVUnionPredicate(NewPreds, *SE);
+          LLVM_DEBUG(dbgs() << "\t    Added runtime assumption: " << *Coeff
+                            << " != 0\n");
+        } else {
+          // Cannot add runtime assumptions, this test cannot handle this case.
+          // Let more complex tests try.
+          LLVM_DEBUG(dbgs() << "\t    Would need runtime assumption " << *Coeff
+                            << " != 0, but not allowed. Failing this test.\n");
+          return false;
+        }
+      } else {
+        LLVM_DEBUG(
+            dbgs() << "\t    Coefficient proven non-zero by SCEV analysis\n");
+      }
+    }
+    // Since 0/X == 0 (where X is known non-zero or assumed non-zero).
     Result.DV[Level].Distance = Delta;
     Result.DV[Level].Direction &= Dependence::DVEntry::EQ;
     ++StrongSIVsuccesses;
@@ -2408,7 +2440,8 @@ bool DependenceInfo::symbolicRDIVtest(const SCEV *A1, const SCEV *A2,
 //
 // Return true if dependence disproved.
 bool DependenceInfo::testSIV(const SCEV *Src, const SCEV *Dst, unsigned &Level,
-                             FullDependence &Result) const {
+                             FullDependence &Result,
+                             bool UnderRuntimeAssumptions) {
   LLVM_DEBUG(dbgs() << "    src = " << *Src << "\n");
   LLVM_DEBUG(dbgs() << "    dst = " << *Dst << "\n");
   const SCEVAddRecExpr *SrcAddRec = dyn_cast<SCEVAddRecExpr>(Src);
@@ -2426,8 +2459,9 @@ bool DependenceInfo::testSIV(const SCEV *Src, const SCEV *Dst, unsigned &Level,
     Level = mapSrcLoop(CurSrcLoop);
     bool disproven;
     if (SrcCoeff == DstCoeff)
-      disproven = strongSIVtest(SrcCoeff, SrcConst, DstConst, CurSrcLoop,
-                                CurDstLoop, Level, Result);
+      disproven =
+          strongSIVtest(SrcCoeff, SrcConst, DstConst, CurSrcLoop, CurDstLoop,
+                        Level, Result, UnderRuntimeAssumptions);
     else if (SrcCoeff == SE->getNegativeSCEV(DstCoeff))
       disproven = weakCrossingSIVtest(SrcCoeff, SrcConst, DstConst, CurSrcLoop,
                                       CurDstLoop, Level, Result);
@@ -3666,6 +3700,10 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
 
   FullDependence Result(Src, Dst, SCEVUnionPredicate(Assume, *SE),
                         PossiblyLoopIndependent, CommonLevels);
+  // Track assumptions before running dependence tests. If new assumptions are
+  // added during tests, we must return the result even if AllEqual to preserve
+  // those assumptions for the caller.
+  size_t AssumptionsBeforeTests = Result.Assumptions.getPredicates().size();
   ++TotalArrayPairs;
 
   for (unsigned P = 0; P < Pairs; ++P) {
@@ -3709,7 +3747,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
     case Subscript::SIV: {
       LLVM_DEBUG(dbgs() << ", SIV\n");
       unsigned Level;
-      if (testSIV(Pair[SI].Src, Pair[SI].Dst, Level, Result))
+      if (testSIV(Pair[SI].Src, Pair[SI].Dst, Level, Result,
+                  UnderRuntimeAssumptions))
         return nullptr;
       break;
     }
@@ -3790,14 +3829,18 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
   } else {
     // On the other hand, if all directions are equal and there's no
     // loop-independent dependence possible, then no dependence exists.
+    // However, if we added runtime assumptions during the dependence tests,
+    // we must return the result to preserve those assumptions for the caller.
     bool AllEqual = true;
     for (unsigned II = 1; II <= CommonLevels; ++II) {
       if (Result.getDirection(II) != Dependence::DVEntry::EQ) {
         AllEqual = false;
         break;
       }
     }
-    if (AllEqual)
+    bool AddedAssumptionsDuringTests =
+        Result.Assumptions.getPredicates().size() > AssumptionsBeforeTests;
+    if (AllEqual && !AddedAssumptionsDuringTests)
       return nullptr;
   }
 

llvm/test/Analysis/DependenceAnalysis/DADelin.ll

@@ -646,11 +646,15 @@ exit:
 define void @coeff_may_negative(ptr %a, i32 %k) {
 ; CHECK-LABEL: 'coeff_may_negative'
 ; CHECK-NEXT:  Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - consistent output [0]!
+; CHECK-NEXT:    Runtime Assumptions:
+; CHECK-NEXT:    Compare predicate: %k ne) 0
 ; CHECK-NEXT:  Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
 ; CHECK-NEXT:    da analyze - 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!
+; CHECK-NEXT:    da analyze - consistent output [0]!
+; CHECK-NEXT:    Runtime Assumptions:
+; CHECK-NEXT:    Compare predicate: %k ne) 0
 ;
 entry:
   br label %loop
@@ -685,11 +689,15 @@ exit:
 define void @coeff_positive(ptr %a, i32 %k) {
 ; CHECK-LABEL: 'coeff_positive'
 ; CHECK-NEXT:  Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - consistent output [0]!
+; CHECK-NEXT:    Runtime Assumptions:
+; CHECK-NEXT:    Compare predicate: %k ne) 0
 ; CHECK-NEXT:  Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
 ; CHECK-NEXT:    da analyze - 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!
+; CHECK-NEXT:    da analyze - consistent output [0]!
+; CHECK-NEXT:    Runtime Assumptions:
+; CHECK-NEXT:    Compare predicate: %k ne) 0
 ;
 entry:
   br label %loop

llvm/test/Analysis/DependenceAnalysis/zero-coefficient.ll

@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
+; RUN: | FileCheck %s
+
+; Test case for bug fix where Strong SIV test incorrectly concludes "no dependence"
+; when the coefficient is symbolic (unknown at compile time) and delta is zero.
+;
+; In this case, the array access is A[k*i] with both src and dst at the same
+; location in the same iteration. If k=0, then all iterations access the same
+; element, meaning there IS a dependence between different iterations.
+; The Strong SIV test should add a runtime assumption that k != 0.
+
+define void @test_zero_coefficient(ptr noalias %A, i64 %k) {
+; CHECK-LABEL: 'test_zero_coefficient'
+; CHECK-NEXT:  Src: store i8 42, ptr %idx, align 1 --> Dst: store i8 42, ptr %idx, align 1
+; CHECK-NEXT:    da analyze - consistent output [0]!
+; CHECK-NEXT:    Runtime Assumptions:
+; CHECK-NEXT:    Compare predicate: %k ne) 0
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+  %off = mul nsw i64 %i, %k
+  %idx = getelementptr inbounds i8, ptr %A, i64 %off
+  store i8 42, ptr %idx, align 1
+  %i.next = add nsw i64 %i, 1
+  %cmp = icmp slt i64 %i.next, 100
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret void
+}