[LoopSimplifyCFG] Change logic of dead loops removal to avoid hitting asserts

The function `LI.erase` has some invariants that need to be preserved when it
tries to remove a loop which is not the top-level loop. In particular, it
requires loop's preheader to be strictly in loop's parent. Our current logic
of deletion of dead blocks may erase the information about preheader before we
handle the loop, and therefore we may hit this assertion.

This patch changes the logic of loop deletion: we make them top-level loops
before we actually erase them. This allows us to trigger the simple branch of
`erase` logic which just detatches blocks from the loop and does not try to do
some complex stuff that need this invariant.

Thanks to @uabelho for reporting this!

Differential Revision: https://reviews.llvm.org/D57221
Reviewed By: fedor.sergeev

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353813 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/LoopSimplifyCFG.cpp b/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
index 5bb6049c3ac6ed1e27ee79673d2530d7e0ad9d5c..4058c1d685cf27247ef1aeea5c9bbe2a0c9c4c39 100644 (file)
--- a/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
+++ b/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
@@ -406,15 +406,32 @@ private:
                                                      DeadLoopBlocks.end());
       MSSAU->removeBlocks(DeadLoopBlocksSet);
     }
+
+    // The function LI.erase has some invariants that need to be preserved when
+    // it tries to remove a loop which is not the top-level loop. In particular,
+    // it requires loop's preheader to be strictly in loop's parent. We cannot
+    // just remove blocks one by one, because after removal of preheader we may
+    // break this invariant for the dead loop. So we detatch and erase all dead
+    // loops beforehand.
+    for (auto *BB : DeadLoopBlocks)
+      if (LI.isLoopHeader(BB)) {
+        assert(LI.getLoopFor(BB) != &L && "Attempt to remove current loop!");
+        Loop *DL = LI.getLoopFor(BB);
+        if (DL->getParentLoop()) {
+          for (auto *PL = DL->getParentLoop(); PL; PL = PL->getParentLoop())
+            for (auto *BB : DL->getBlocks())
+              PL->removeBlockFromLoop(BB);
+          DL->getParentLoop()->removeChildLoop(DL);
+          LI.addTopLevelLoop(DL);
+        }
+        LI.erase(DL);
+      }
+
     for (auto *BB : DeadLoopBlocks) {
       assert(BB != L.getHeader() &&
              "Header of the current loop cannot be dead!");
       LLVM_DEBUG(dbgs() << "Deleting dead loop block " << BB->getName()
                         << "\n");
-      if (LI.isLoopHeader(BB)) {
-        assert(LI.getLoopFor(BB) != &L && "Attempt to remove current loop!");
-        LI.erase(LI.getLoopFor(BB));
-      }
       LI.removeBlock(BB);
     }
 

diff --git a/test/Transforms/LoopSimplifyCFG/update_parents.ll b/test/Transforms/LoopSimplifyCFG/update_parents.ll
index e9a25c9151fa5ab7aa55bb5feb534db34104f309..24da74557412e74e2d4ed37a3cb35ec9ff03d327 100644 (file)
--- a/test/Transforms/LoopSimplifyCFG/update_parents.ll
+++ b/test/Transforms/LoopSimplifyCFG/update_parents.ll
@@ -1,5 +1,4 @@
-; XFAIL: *
-; REQUIRES: asserts
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -enable-loop-simplifycfg-term-folding=true -loop-simplifycfg -debug-only=loop-simplifycfg -verify-loop-info -verify-dom-info -verify-loop-lcssa 2>&1 < %s | FileCheck %s
 ; RUN: opt -S -enable-loop-simplifycfg-term-folding=true -passes='require<domtree>,loop(simplify-cfg)' -debug-only=loop-simplifycfg -verify-loop-info -verify-dom-info -verify-loop-lcssa 2>&1 < %s | FileCheck %s
 ; RUN: opt -S -enable-loop-simplifycfg-term-folding=true -loop-simplifycfg -enable-mssa-loop-dependency=true -verify-memoryssa -debug-only=loop-simplifycfg -verify-loop-info -verify-dom-info -verify-loop-lcssa 2>&1 < %s | FileCheck %s
@@ -7,8 +6,24 @@
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @test() {
-
 ; CHECK-LABEL: @test(
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1.loopexit:
+; CHECK-NEXT:    br label [[BB1]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2.loopexit:
+; CHECK-NEXT:    br label [[BB2]]
+; CHECK:       bb2:
+; CHECK-NEXT:    switch i32 0, label [[BB2_SPLIT:%.*]] [
+; CHECK-NEXT:    i32 1, label [[BB1_LOOPEXIT:%.*]]
+; CHECK-NEXT:    i32 2, label [[BB2_LOOPEXIT:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       bb2-split:
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb3:
+; CHECK-NEXT:    br label [[BB3]]
+;
 
   br label %bb1
 
@@ -32,8 +47,24 @@ bb8:                                              ; preds = %bb6
 }
 
 define void @test_many_subloops(i1 %c) {
-
 ; CHECK-LABEL: @test_many_subloops(
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1.loopexit:
+; CHECK-NEXT:    br label [[BB1]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2.loopexit:
+; CHECK-NEXT:    br label [[BB2]]
+; CHECK:       bb2:
+; CHECK-NEXT:    switch i32 0, label [[BB2_SPLIT:%.*]] [
+; CHECK-NEXT:    i32 1, label [[BB1_LOOPEXIT:%.*]]
+; CHECK-NEXT:    i32 2, label [[BB2_LOOPEXIT:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       bb2-split:
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb3:
+; CHECK-NEXT:    br label [[BB3]]
+;
 
   br label %bb1