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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: rupprecht, uabelho, anna, skatkov.

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!


https://reviews.llvm.org/D57221

Files:
  lib/Transforms/Scalar/LoopSimplifyCFG.cpp
  test/Transforms/LoopSimplifyCFG/update_parents.ll


Index: test/Transforms/LoopSimplifyCFG/update_parents.ll
===================================================================
--- test/Transforms/LoopSimplifyCFG/update_parents.ll
+++ test/Transforms/LoopSimplifyCFG/update_parents.ll
@@ -1,4 +1,4 @@
-; XFAIL: *
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; REQUIRES: asserts
 ; 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
@@ -7,8 +7,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 +48,24 @@
 }
 
 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
 
Index: lib/Transforms/Scalar/LoopSimplifyCFG.cpp
===================================================================
--- lib/Transforms/Scalar/LoopSimplifyCFG.cpp
+++ lib/Transforms/Scalar/LoopSimplifyCFG.cpp
@@ -401,15 +401,24 @@
                                                      DeadLoopBlocks.end());
       MSSAU->removeBlocks(DeadLoopBlocksSet);
     }
+    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);
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57221.183494.patch
Type: text/x-patch
Size: 3452 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190125/7664bf17/attachment.bin>