[llvm] 3c74ed9 - [LAA] Fix ICE with scAddExpr in forked pointers

[Graham Hunter via llvm-commits]

Author: Graham Hunter
Date: 2022-09-21T10:27:06+01:00
New Revision: 3c74ed9ee3233d4272a7b213091a9fd1cb464d71

URL: https://github.com/llvm/llvm-project/commit/3c74ed9ee3233d4272a7b213091a9fd1cb464d71
DIFF: https://github.com/llvm/llvm-project/commit/3c74ed9ee3233d4272a7b213091a9fd1cb464d71.diff

LOG: [LAA] Fix ICE with scAddExpr in forked pointers

The IR from https://github.com/llvm/llvm-project/issues/57368 results
in an assert firing when trying to create a runtime check for the
forked pointer. One of the forks is fine since it's loop invariant,
but the other is a scAddExpr (containing a scAddRecExpr, so not
invariant) when RtCheck::insert expects a scAddRecExpr.

This is a simple fix to just avoid forks which aren't AddRec or
loop invariant. We can allow it as a forked pointer later with
more work.

Reviewed By: fhahn

Differential Revision: https://reviews.llvm.org/D133020

Added: 
    

Modified: 
    llvm/lib/Analysis/LoopAccessAnalysis.cpp
    llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index dc56d8d019b21..f06e412352859 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -954,9 +954,18 @@ findForkedPointer(PredicatedScalarEvolution &PSE,
   SmallVector<std::pair<const SCEV *, bool>> Scevs;
   findForkedSCEVs(SE, L, Ptr, Scevs, MaxForkedSCEVDepth);
 
-  // For now, we will only accept a forked pointer with two possible SCEVs.
-  if (Scevs.size() == 2)
+  // For now, we will only accept a forked pointer with two possible SCEVs
+  // that are either SCEVAddRecExprs or loop invariant.
+  if (Scevs.size() == 2 &&
+      (isa<SCEVAddRecExpr>(Scevs[0].first) ||
+       SE->isLoopInvariant(Scevs[0].first, L)) &&
+      (isa<SCEVAddRecExpr>(Scevs[1].first) ||
+       SE->isLoopInvariant(Scevs[1].first, L))) {
+    LLVM_DEBUG(dbgs() << "LAA: Found forked pointer: " << *Ptr << "\n");
+    LLVM_DEBUG(dbgs() << "\t(1) " << *(Scevs[0].first) << "\n");
+    LLVM_DEBUG(dbgs() << "\t(2) " << *(Scevs[1].first) << "\n");
     return Scevs;
+  }
 
   return {
       std::make_pair(replaceSymbolicStrideSCEV(PSE, StridesMap, Ptr), false)};

diff  --git a/llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll b/llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll
index efde509d75cf8..3f5c8e4c1c72f 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll
@@ -569,13 +569,13 @@ for.body:                                         ; preds = %entry, %for.body
 ; CHECK-NEXT:      Against group ([[G2:.+]]):
 ; CHECK-NEXT:        %arrayidx = getelementptr inbounds i32, ptr %Preds, i64 %indvars.iv
 ; CHECK-NEXT:    Check 1:
-; CHECK-NEXT:      Comparing group ([[G1:.+]]):
+; CHECK-NEXT:      Comparing group ([[G1]]):
 ; CHECK-NEXT:        %arrayidx5 = getelementptr inbounds float, ptr %Dest, i64 %indvars.iv
 ; CHECK-NEXT:      Against group ([[G3:.+]]):
 ; CHECK-NEXT:        %arrayidx3 = getelementptr inbounds float, ptr %Base, i64 %offset
 ; CHECK-NEXT:        %arrayidx3 = getelementptr inbounds float, ptr %Base, i64 %offset
 ; CHECK-NEXT:    Grouped accesses:
-; CHECK-NEXT:      Group [[G1:.+]]:
+; CHECK-NEXT:      Group [[G1]]:
 ; CHECK-NEXT:        (Low: %Dest High: (400 + %Dest))
 ; CHECK-NEXT:          Member: {%Dest,+,4}<nuw><%for.body>
 ; CHECK-NEXT:      Group [[G2]]:
@@ -880,3 +880,59 @@ for.body:
 for.cond.cleanup:
   ret void
 }
+
+; CHECK-LABEL: Loop access info in function 'sc_add_expr_ice':
+; CHECK-NEXT:   for.body:
+; CHECK-NEXT:     Memory dependences are safe with run-time checks
+; CHECK-NEXT:     Dependences:
+; CHECK-NEXT:     Run-time memory checks:
+; CHECK-NEXT:     Check 0:
+; CHECK-NEXT:       Comparing group ([[G1:.+]]):
+; CHECK-NEXT:       ptr %Base1
+; CHECK-NEXT:       Against group ([[G2:.+]]):
+; CHECK-NEXT:         %fptr = getelementptr inbounds double, ptr %Base2, i64 %sel
+; CHECK-NEXT:     Grouped accesses:
+; CHECK-NEXT:       Group [[G1]]:
+; CHECK-NEXT:         (Low: %Base1 High: (8 + %Base1))
+; CHECK-NEXT:           Member: %Base1
+; CHECK-NEXT:       Group [[G2]]:
+; CHECK-NEXT:         (Low: %Base2 High: ((8 * %N) + %Base2))
+; CHECK-NEXT:           Member: {%Base2,+,8}<%for.body>
+; CHECK-EMPTY:
+; CHECK-NEXT:     Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:     SCEV assumptions:
+; CHECK-NEXT:     {0,+,1}<%for.body> Added Flags: <nusw>
+; CHECK-EMPTY:
+; CHECK-NEXT:     Expressions re-written:
+; CHECK-NEXT:     [PSE]  %fptr = getelementptr inbounds double, ptr %Base2, i64 %sel:
+; CHECK-NEXT:       ((8 * (zext i32 {0,+,1}<%for.body> to i64))<nuw><nsw> + %Base2)<nuw>
+; CHECK-NEXT:       --> {%Base2,+,8}<%for.body>
+
+;;; The following test caused an ICE with the initial forked pointers work.
+;;; One fork is loop invariant (%Base2 + 0), the other is an scAddExpr that
+;;; contains an scAddRecExpr inside it:
+;;;   ((8 * (zext i32 {0,+,1}<%for.body> to i64))<nuw><nsw> + %Base2)<nuw>
+;;;
+;;; RtCheck::insert was expecting either loop invariant or SAR, so asserted
+;;; on a plain scAddExpr. For now we restrict to loop invariant or SAR
+;;; forks only, but we should be able to do better.
+
+define void @sc_add_expr_ice(ptr %Base1, ptr %Base2, i64 %N) {
+entry:
+  br label %for.body
+
+for.body:
+  %iv = phi i64 [ %iv.next, %for.body ], [ 0, %entry ]
+  %iv.trunc = trunc i64 %iv to i32
+  store double 0.000000e+00, ptr %Base1, align 8
+  %iv.zext = zext i32 %iv.trunc to i64
+  %sel = select i1 true, i64 %iv.zext, i64 0
+  %fptr = getelementptr inbounds double, ptr %Base2, i64 %sel
+  %dummy.load = load double, ptr %fptr, align 8
+  %iv.next = add nuw nsw i64 %iv, 1
+  %exitcond = icmp eq i64 %iv.next, %N
+  br i1 %exitcond, label %exit, label %for.body
+
+exit:
+  ret void
+}