[llvm] ae7b1e8 - [SCEV] Handle unreachable binop when matching shift recurrence

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-03-31T10:33:34-07:00
New Revision: ae7b1e8823a51068cfa64875fc5222e5b1d16760

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

LOG: [SCEV] Handle unreachable binop when matching shift recurrence

This fixes an issue introduced with my change d4648e, and reported in pr49768.

The root problem is that dominance collapses in unreachable code, and that LoopInfo explicitly only models reachable code.  Since the recurrence matcher doesn't filter by reachability (and can't easily because not all consumers have domtree), we need to bailout before assuming that finding a recurrence implies we found a loop.

Added: 
    

Modified: 
    llvm/lib/Analysis/ScalarEvolution.cpp
    llvm/test/Analysis/ScalarEvolution/shift-recurrences.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index d0700399e5066..5a3aa82113da8 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -5669,13 +5669,16 @@ getRangeForUnknownRecurrence(const SCEVUnknown *U) {
   if (!matchSimpleRecurrence(P, BO, Start, Step))
     return CR;
 
-  // If we found a recurrence, we must be in a loop -- unless we're
-  // in unreachable code where dominance collapses.  Note that BO might
-  // be in some subloop of L, and that's completely okay.
-  auto *L = LI.getLoopFor(P->getParent());
-  if (!L)
+  if (!DT.isReachableFromEntry(P->getParent()) ||
+      !DT.isReachableFromEntry(BO->getParent()))
+    // If either is in unreachable code, dominance collapses and none of our
+    // expected post conditions about loops hold.
     return CR;
-  assert(L->getHeader() == P->getParent());
+
+  // If we found a recurrence in reachable code, we must be in a loop. Note
+  // that BO might be in some subloop of L, and that's completely okay.
+  auto *L = LI.getLoopFor(P->getParent());
+  assert(L && L->getHeader() == P->getParent());
   if (!L->contains(BO->getParent()))
     // NOTE: This bailout should be an assert instead.  However, asserting
     // the condition here exposes a case where LoopFusion is querying SCEV

diff  --git a/llvm/test/Analysis/ScalarEvolution/shift-recurrences.ll b/llvm/test/Analysis/ScalarEvolution/shift-recurrences.ll
index 5f34ad42ab5f6..4d24e02afdad5 100644
--- a/llvm/test/Analysis/ScalarEvolution/shift-recurrences.ll
+++ b/llvm/test/Analysis/ScalarEvolution/shift-recurrences.ll
@@ -383,3 +383,60 @@ loop:
 exit:
   ret void
 }
+
+; Corner case where phi is not in a loop because it is in unreachable
+; code (which loopinfo ignores, but simple recurrence matching does not).
+define void @unreachable_phi() {
+; CHECK-LABEL: 'unreachable_phi'
+; CHECK-NEXT:  Classifying expressions for: @unreachable_phi
+; CHECK-NEXT:    %p_58.addr.1 = phi i32 [ undef, %unreachable1 ], [ %sub2629, %unreachable2 ]
+; CHECK-NEXT:    --> undef U: full-set S: full-set
+; CHECK-NEXT:    %sub2629 = sub i32 %p_58.addr.1, 1
+; CHECK-NEXT:    --> undef U: full-set S: full-set
+; CHECK-NEXT:  Determining loop execution counts for: @unreachable_phi
+;
+entry:
+  ret void
+
+unreachable1:
+  br label %unreachable_nonloop
+unreachable2:
+  br label %unreachable_nonloop
+unreachable_nonloop:
+  %p_58.addr.1 = phi i32 [ undef, %unreachable1 ], [ %sub2629, %unreachable2 ]
+  %sub2629 = sub i32 %p_58.addr.1, 1
+  unreachable
+}
+
+; Corner case where phi is not in loop header because binop is in unreachable
+; code (which loopinfo ignores, but simple recurrence matching does not).
+define void @unreachable_binop() {
+; CHECK-LABEL: 'unreachable_binop'
+; CHECK-NEXT:  Classifying expressions for: @unreachable_binop
+; CHECK-NEXT:    %p_58.addr.1 = phi i32 [ undef, %header ], [ %sub2629, %unreachable ]
+; CHECK-NEXT:    --> %p_58.addr.1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %header: Variant }
+; CHECK-NEXT:    %sub2629 = sub i32 %p_58.addr.1, 1
+; CHECK-NEXT:    --> undef U: full-set S: full-set
+; CHECK-NEXT:  Determining loop execution counts for: @unreachable_binop
+; CHECK-NEXT:  Loop %header: Unpredictable backedge-taken count.
+; CHECK-NEXT:  Loop %header: Unpredictable max backedge-taken count.
+; CHECK-NEXT:  Loop %header: Unpredictable predicated backedge-taken count.
+;
+entry:
+  br label %header
+
+header:
+  br label %for.cond2295
+
+for.cond2295:
+  %p_58.addr.1 = phi i32 [ undef, %header ], [ %sub2629, %unreachable ]
+  br i1 undef, label %if.then2321, label %header
+
+if.then2321:
+  ret void
+
+unreachable:
+  %sub2629 = sub i32 %p_58.addr.1, 1
+  br label %for.cond2295
+}
+