[DA] Fix the check between Subscript and Size after delinearization (#151326)

Delinearization provides two values: the size of the array, and the
subscript of the access. DA checks their validity (`0 <= subscript <
size`), with some special handling. In particular, to ensure `subscript
< size`, calculate the maximum value of `subscript - size` and check if
it is negative. There was an issue in its process: when `subscript -
size` is expressed as an affine format like `init + step * i`, the value
in the last iteration (`start + step * (num_iterations - 1)`) was
assumed to be the maximum value. This assumption is incorrect in the
following cases:

- When `step` is negative
- When the AddRec wraps

This patch introduces extra checks to ensure the sign of `step` and
verify the existence of nsw/nuw flags.

Also, `isKnownNonNegative(S - smax(1, Size))` was used as a regular
check, which is incorrect when `Size` is negative. This patch also
replace it with `isKnownNonNegative(S - Size)`, although it's still
unclear whether using `isKnownNonNegative` is appropriate in the first
place.

Fix #150604

Author: kasuga-fj

llvm/lib/Analysis/DependenceAnalysis.cpp

@@ -1074,7 +1074,7 @@ bool DependenceInfo::isKnownPredicate(ICmpInst::Predicate Pred, const SCEV *X,
 
 /// Compare to see if S is less than Size, using
 ///
-///    isKnownNegative(S - max(Size, 1))
+///    isKnownNegative(S - Size)
 ///
 /// with some extra checking if S is an AddRec and we can prove less-than using
 /// the loop bounds.
@@ -1090,21 +1090,34 @@ bool DependenceInfo::isKnownLessThan(const SCEV *S, const SCEV *Size) const {
   Size = SE->getTruncateOrZeroExtend(Size, MaxType);
 
   // Special check for addrecs using BE taken count
-  const SCEV *Bound = SE->getMinusSCEV(S, Size);
-  if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Bound)) {
-    if (AddRec->isAffine()) {
+  if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(S))
+    if (AddRec->isAffine() && AddRec->hasNoSignedWrap()) {
       const SCEV *BECount = SE->getBackedgeTakenCount(AddRec->getLoop());
-      if (!isa<SCEVCouldNotCompute>(BECount)) {
-        const SCEV *Limit = AddRec->evaluateAtIteration(BECount, *SE);
-        if (SE->isKnownNegative(Limit))
-          return true;
-      }
+      const SCEV *Start = AddRec->getStart();
+      const SCEV *Step = AddRec->getStepRecurrence(*SE);
+      const SCEV *End = AddRec->evaluateAtIteration(BECount, *SE);
+      const SCEV *Diff0 = SE->getMinusSCEV(Start, Size);
+      const SCEV *Diff1 = SE->getMinusSCEV(End, Size);
+
+      // If the value of Step is non-negative and the AddRec is non-wrap, it
+      // reaches its maximum at the last iteration. So it's enouth to check
+      // whether End - Size is negative.
+      if (SE->isKnownNonNegative(Step) && SE->isKnownNegative(Diff1))
+        return true;
+
+      // If the value of Step is non-positive and the AddRec is non-wrap, the
+      // initial value is its maximum.
+      if (SE->isKnownNonPositive(Step) && SE->isKnownNegative(Diff0))
+        return true;
+
+      // Even if we don't know the sign of Step, either Start or End must be
+      // the maximum value of the AddRec since it is non-wrap.
+      if (SE->isKnownNegative(Diff0) && SE->isKnownNegative(Diff1))
+        return true;
     }
-  }
 
   // Check using normal isKnownNegative
-  const SCEV *LimitedBound =
-      SE->getMinusSCEV(S, SE->getSMaxExpr(Size, SE->getOne(Size->getType())));
+  const SCEV *LimitedBound = SE->getMinusSCEV(S, Size);
   return SE->isKnownNegative(LimitedBound);
 }
 

llvm/test/Analysis/DDG/basic-loopnest.ll

@@ -1,5 +1,8 @@
 ; RUN: opt < %s -disable-output "-passes=print<ddg>" 2>&1 | FileCheck %s
 
+; XFAIL: *
+; At the moment, DependenceAnalysis cannot infer `n` to be positive.
+
 
 ; CHECK-LABEL: 'DDG' for loop 'test1.for.cond1.preheader':
 
@@ -378,4 +381,4 @@ for.inc12:                                        ; preds = %for.body4, %test2.f
 
 for.end14:                                        ; preds = %for.inc12, %entry
   ret void
-}
+}

llvm/test/Analysis/DependenceAnalysis/Coupled.ll

@@ -719,12 +719,14 @@ for.end:                                          ; preds = %for.body
 ;;    for(int j = 0; j < M; j+=1)
 ;;      A[M*N + M*i + j] = 2;
 
+; FIXME: Currently failing to infer %M being positive.
+
 define void @couple_weakzerosiv(ptr noalias nocapture %A, i64 %N, i64 %M) {
 ; CHECK-LABEL: 'couple_weakzerosiv'
 ; CHECK-NEXT:  Src: store i32 1, ptr %arrayidx.us, align 4 --> Dst: store i32 1, ptr %arrayidx.us, align 4
 ; CHECK-NEXT:    da analyze - none!
 ; CHECK-NEXT:  Src: store i32 1, ptr %arrayidx.us, align 4 --> Dst: store i32 2, ptr %arrayidx9.us, align 4
-; CHECK-NEXT:    da analyze - output [p>]!
+; CHECK-NEXT:    da analyze - output [*|<]!
 ; CHECK-NEXT:  Src: store i32 2, ptr %arrayidx9.us, align 4 --> Dst: store i32 2, ptr %arrayidx9.us, align 4
 ; CHECK-NEXT:    da analyze - none!
 ;

llvm/test/Analysis/DependenceAnalysis/DADelin.ll

@@ -594,14 +594,15 @@ for.end12:                                        ; preds = %for.inc10, %entry
 }
 
 
+; FIXME? It seems that we cannot prove that %N is non-negative...
 define void @nonnegative(ptr nocapture %A, i32 %N) {
 ; CHECK-LABEL: 'nonnegative'
 ; CHECK-NEXT:  Src: store i32 1, ptr %arrayidx, align 4 --> Dst: store i32 1, ptr %arrayidx, align 4
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - output [* *]!
 ; CHECK-NEXT:  Src: store i32 1, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
-; CHECK-NEXT:    da analyze - consistent output [0 0|<]!
+; CHECK-NEXT:    da analyze - output [* *|<]!
 ; CHECK-NEXT:  Src: store i32 2, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - output [* *]!
 ;
 entry:
   %cmp44 = icmp eq i32 %N, 0
@@ -630,3 +631,81 @@ for.latch:
 exit:
   ret void
 }
+
+; i = 0;
+; do {
+;   a[k * i] = 42;
+;   a[k * (i + 1)] = 42;
+;   i++;
+; } while (i != k);
+;
+; The dependency direction between the two stores depends on the sign of k.
+; Note that the loop guard is omitted intentionally.
+; FIXME: Each store has loop-carried dependencies on itself if k is zero.
+;
+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:  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!
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
+  %i.next = add i32 %i, 1
+  %subscript.0 = mul i32 %i, %k
+  %subscript.1 = mul i32 %i.next, %k
+  %idx.0 = getelementptr i8, ptr %a, i32 %subscript.0
+  %idx.1 = getelementptr i8, ptr %a, i32 %subscript.1
+  store i8 42, ptr %idx.0
+  store i8 42, ptr %idx.1
+  %cond.exit = icmp eq i32 %i.next, %k
+  br i1 %cond.exit, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+; i = 0;
+; do {
+;   a[k * i] = 42;
+;   a[k * (i + 1)] = 42;
+;   i++;
+; } while (i != k);
+;
+; Note that the loop guard is omitted intentionally.
+; FIXME: In principle, we can infer that the value of k is non-negative from
+; the nsw flag.
+;
+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:  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!
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
+  %i.next = add nsw i32 %i, 1
+  %subscript.0 = mul i32 %i, %k
+  %subscript.1 = mul i32 %i.next, %k
+  %idx.0 = getelementptr i8, ptr %a, i32 %subscript.0
+  %idx.1 = getelementptr i8, ptr %a, i32 %subscript.1
+  store i8 42, ptr %idx.0
+  store i8 42, ptr %idx.1
+  %cond.exit = icmp eq i32 %i.next, %k
+  br i1 %cond.exit, label %exit, label %loop
+
+exit:
+  ret void
+}