[SCEV] Handle unreachable binop when matching shift recurrence
authorPhilip Reames <listmail@philipreames.com>
Wed, 31 Mar 2021 17:29:21 +0000 (10:29 -0700)
committerPhilip Reames <listmail@philipreames.com>
Wed, 31 Mar 2021 17:33:34 +0000 (10:33 -0700)
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.

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

index d070039..5a3aa82 100644 (file)
@@ -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
index 5f34ad4..4d24e02 100644 (file)
@@ -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
+}
+