[PATCH] D41519: [BasicBlockUtils] Check for unreachable preds before updating LI in UpdateAnalysisInformation

[Anna Thomas via Phabricator via llvm-commits]

anna created this revision.
anna added reviewers: davide, chandlerc, sanjoy.

We are incorrectly updating the LI when loop-simplify generates
dedicated exit blocks for a loop. The issue is that there's an implicit
assumption that the Preds passed into UpdateAnalysisInformation are
reachable. However, this is not true and breaks LI by incorrectly
updating the header of a loop.

One such case is when we generate dedicated exits when the exit block is
a landing pad (through SplitLandingPadPredecessors). There maybe other
cases as well, since we do not guarantee that Preds passed in are
reachable basic blocks.

The added test case shows how loop-simplify breaks LI for the outer loop (and DT in turn)
after we try to generate the LoopSimplifyForm.


https://reviews.llvm.org/D41519

Files:
  lib/Transforms/Utils/BasicBlockUtils.cpp
  test/Transforms/LoopSimplify/unreachable-loop-pred.ll


Index: test/Transforms/LoopSimplify/unreachable-loop-pred.ll
===================================================================
--- test/Transforms/LoopSimplify/unreachable-loop-pred.ll
+++ test/Transforms/LoopSimplify/unreachable-loop-pred.ll
@@ -18,3 +18,74 @@
 foo:
   br label %while.body115
 }
+
+; When loopsimplify generates dedicated exit blocks for blocks that are landing
+; pads, we should not incorrectly mark that exit block as the header for the outer
+; loop. This happens when it gets confused with the unreachable pred to the inner
+; loop exit block
+define align 8 void @baz() personality i32* ()* @wobble {
+bb:
+  br label %outerH
+
+outerH:
+  %tmp30 = invoke align 8 i8 * undef()
+          to label %outerLExiting1 unwind label %unwindblock
+
+unwindblock:
+  %tmp34 = landingpad { i8*, i32 }
+          cleanup
+  ret void
+
+outerLExiting1:
+  br i1 undef, label %outerLExiting2, label %exit
+
+exit:
+  ret void
+
+outerLExiting2:
+  invoke void @foo() 
+          to label %innerPreheader unwind label %innerLExit
+
+innerPreheader:                                             ; preds = %outerLExiting2
+  br label %innerH
+
+innerH:                                             ; preds = %innerLatch, %innerPreheader
+  %tmp50 = invoke i8 * undef()
+          to label %innerExiting unwind label %innerLExit
+
+innerExiting:                                             ; preds = %bb57
+  invoke align 8 void @foo() 
+          to label %innerLatch unwind label %innerLExit
+
+innerLatch:                                             ; preds = %innerExiting
+  br label %innerH
+
+; This is a predecessor for inner loop exit block which is a landing pad.
+unreachableB:                                             ; No predecessors!
+  %tmp62 = invoke i8 * undef()
+          to label %unwindblock2 unwind label %innerLExit
+
+unwindblock2:                                             ; preds = %unreachableB
+  ret void
+
+innerLExit:
+  %tmp65 = landingpad { i8*, i32 }
+          cleanup
+  invoke void @foo() 
+          to label %outerLatch unwind label %bb66
+
+bb66:                                             ; preds = %innerLExit
+  %tmp67 = landingpad { i8*, i32 }
+          cleanup
+  ret void
+
+
+outerLatch:                                             ; preds = %bb70
+  br label %outerH
+}
+
+; Function Attrs: nounwind
+declare i32* @wobble()
+
+; Function Attrs: uwtable
+declare void @foo()
Index: lib/Transforms/Utils/BasicBlockUtils.cpp
===================================================================
--- lib/Transforms/Utils/BasicBlockUtils.cpp
+++ lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -331,6 +331,13 @@
   bool IsLoopEntry = !!L;
   bool SplitMakesNewLoopHeader = false;
   for (BasicBlock *Pred : Preds) {
+    // Preds that are not reachable from entry should not be used to identify if
+    // OldBB is a loop entry or if SplitMakesNewLoopHeader. Blocks that do not
+    // have any predecessors are not within any loops, so we incorrectly mark
+    // SplitMakesNewLoopHeader as true and make the NewBB the header of some
+    // loop. This breaks LI.
+    if (!DT->isReachableFromEntry(Pred))
+      continue;
     // If we need to preserve LCSSA, determine if any of the preds is a loop
     // exit.
     if (PreserveLCSSA)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41519.127948.patch
Type: text/x-patch
Size: 3293 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171221/850fe9d4/attachment.bin>