[DA] Properly pass outermost loop to monotonicity checker

[kasuga-fj]

This patch fixes the unexpected result in monotonicity check for @step_is_variant in monotonicity-no-wrap-flags.ll. Currently, the SCEV is considered non-monotonic if it contains an expression which is neither loop-invariant nor an affine addrec. In @step_is_variant, the offset_i satisfies this condition, but offset_i + j was classified as monotonic.

The root cause is that a non-outermost loop was passed to monotonicity checker instead of the outermost one. This patch ensures that the correct outermost loop is passed.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

This patch fixes the unexpected result in monotonicity check for @<!-- -->step_is_variant in monotonicity-no-wrap-flags.ll. Currently, the SCEV is considered non-monotonic if it contains an expression which is neither loop-invariant nor an affine addrec. In @<!-- -->step_is_variant, the offset_i satisfies this condition, but offset_i + j was classified as monotonic.

The root cause is that a non-outermost loop was passed to monotonicity checker instead of the outermost one. This patch ensures that the correct outermost loop is passed.

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+4-1)
• (modified) llvm/test/Analysis/DependenceAnalysis/monotonicity-no-wrap-flags.ll (+54-2)

diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index e45d1f79b3165..b3b62cfe8b459 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -407,9 +407,10 @@ static void dumpExampleDependence(raw_ostream &OS, DependenceInfo *DA,
         continue;
       Value *Ptr = getLoadStorePointerOperand(&Inst);
       const Loop *L = LI.getLoopFor(Inst.getParent());
+      const Loop *OutermostLoop = L ? L->getOutermostLoop() : nullptr;
       const SCEV *PtrSCEV = SE.getSCEVAtScope(Ptr, L);
       const SCEV *AccessFn = SE.removePointerBase(PtrSCEV);
-      SCEVMonotonicity Mon = Checker.checkMonotonicity(AccessFn, L);
+      SCEVMonotonicity Mon = Checker.checkMonotonicity(AccessFn, OutermostLoop);
       OS.indent(2) << "Inst: " << Inst << "\n";
       OS.indent(4) << "Expr: " << *AccessFn << "\n";
       Mon.print(OS, 4);
@@ -945,6 +946,8 @@ SCEVMonotonicity SCEVMonotonicityChecker::invariantOrUnknown(const SCEV *Expr) {
 SCEVMonotonicity
 SCEVMonotonicityChecker::checkMonotonicity(const SCEV *Expr,
                                            const Loop *OutermostLoop) {
+  assert((!OutermostLoop || OutermostLoop->isOutermost()) &&
+         "OutermostLoop must be outermost");
   assert(Expr->getType()->isIntegerTy() && "Expr must be integer type");
   this->OutermostLoop = OutermostLoop;
   return visit(Expr);
diff --git a/llvm/test/Analysis/DependenceAnalysis/monotonicity-no-wrap-flags.ll b/llvm/test/Analysis/DependenceAnalysis/monotonicity-no-wrap-flags.ll
index 7411dc9f5c053..df42c757a3b63 100644
--- a/llvm/test/Analysis/DependenceAnalysis/monotonicity-no-wrap-flags.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/monotonicity-no-wrap-flags.ll
@@ -298,7 +298,8 @@ exit:
 }
 
 ; The value of step reccurence is not invariant with respect to the outer most
-; loop (the i-loop).
+; loop (the i-loop). It is theoretically multivariate monotonic by definition,
+; but we cannot handle non-affine addrec for now.
 ;
 ; offset_i = 0;
 ; for (int i = 0; i < 100; i++) {
@@ -312,7 +313,8 @@ define void @step_is_variant(ptr %a) {
 ; CHECK-NEXT:  Monotonicity check:
 ; CHECK-NEXT:    Inst: store i8 0, ptr %idx, align 1
 ; CHECK-NEXT:      Expr: {%offset.i,+,1}<nuw><nsw><%loop.j>
-; CHECK-NEXT:      Monotonicity: MultivariateSignedMonotonic
+; CHECK-NEXT:      Monotonicity: Unknown
+; CHECK-NEXT:      Reason: %offset.i
 ; CHECK-EMPTY:
 ; CHECK-NEXT:  Src: store i8 0, ptr %idx, align 1 --> Dst: store i8 0, ptr %idx, align 1
 ; CHECK-NEXT:    da analyze - confused!
@@ -346,6 +348,56 @@ exit:
   ret void
 }
 
+; The value of step reccurence is not invariant with respect to the outer most
+; loop (the i-loop). Actually, `offset_i` is not monotonic.
+;
+; offset_i = 0;
+; for (int i = 0; i < 100; i++) {
+;   for (int j = 0; j < 100; j++)
+;     a[offset_i + j] = 0;
+;   offset_i += (i % 2 == 0) ? -1 : 3;
+; }
+;
+define void @step_is_variant2(ptr %a) {
+; CHECK-LABEL: 'step_is_variant2'
+; CHECK-NEXT:  Monotonicity check:
+; CHECK-NEXT:    Inst: store i8 0, ptr %idx, align 1
+; CHECK-NEXT:      Expr: {%offset.i,+,1}<nsw><%loop.j>
+; CHECK-NEXT:      Monotonicity: Unknown
+; CHECK-NEXT:      Reason: %offset.i
+; CHECK-EMPTY:
+; CHECK-NEXT:  Src: store i8 0, ptr %idx, align 1 --> Dst: store i8 0, ptr %idx, align 1
+; CHECK-NEXT:    da analyze - confused!
+;
+entry:
+  br label %loop.i.header
+
+loop.i.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.i.latch ]
+  %offset.i = phi i64 [ 0, %entry ], [ %offset.i.next, %loop.i.latch ]
+  %step.i.0 = phi i64 [ -1, %entry ], [ %step.i.1, %loop.i.latch ]
+  %step.i.1 = phi i64 [ 3, %entry ], [ %step.i.0, %loop.i.latch ]
+  br label %loop.j
+
+loop.j:
+  %j = phi i64 [ 0, %loop.i.header ], [ %j.inc, %loop.j ]
+  %offset = add nsw i64 %offset.i, %j
+  %idx = getelementptr inbounds i8, ptr %a, i64 %offset
+  store i8 0, ptr %idx
+  %j.inc = add nsw i64 %j, 1
+  %exitcond.j = icmp eq i64 %j.inc, 100
+  br i1 %exitcond.j, label %loop.i.latch, label %loop.j
+
+loop.i.latch:
+  %i.inc = add nsw i64 %i, 1
+  %offset.i.next = add nsw i64 %offset.i, %step.i.0
+  %exitcond.i = icmp eq i64 %i.inc, 100
+  br i1 %exitcond.i, label %exit, label %loop.i.header
+
+exit:
+  ret void
+}
+
 ; The AddRec doesn't have nsw flag for the j-loop, since the store may not be
 ; executed.
 ;

[Meinersbur]

LGTM

[Meinersbur]

🤣

[kasuga-fj]

It's a silly assertion, but it was buggy because OutermostLoop was not outermost...