[Dominators] Make RemoveUnreachableBlocks return false if the BasicBlock is already...
authorChijun Sima <simachijun@gmail.com>
Fri, 3 Aug 2018 12:45:29 +0000 (12:45 +0000)
committerChijun Sima <simachijun@gmail.com>
Fri, 3 Aug 2018 12:45:29 +0000 (12:45 +0000)
Summary:
Previously, `removeUnreachableBlocks` still returns true (which indicates the CFG is changed) even when all the unreachable blocks found is awaiting deletion in the DDT class.
This makes code pattern like
```
// Code modified from lib/Transforms/Scalar/SimplifyCFGPass.cpp
bool EverChanged = removeUnreachableBlocks(F, nullptr, DDT);
...
do {
    EverChanged = someMightHappenModifications();
    EverChanged |= removeUnreachableBlocks(F, nullptr, DDT);
  } while (EverChanged);
```
become a dead loop.
Fix this by detecting whether a BasicBlock is already awaiting deletion.

Reviewers: kuhar, brzycki, dmgreen, grosser, davide

Reviewed By: kuhar, brzycki

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D49738

llvm-svn: 338882

llvm/lib/Transforms/Utils/Local.cpp
llvm/unittests/Transforms/Utils/Local.cpp

index 0c3afc4..6bddbb2 100644 (file)
@@ -2264,8 +2264,16 @@ bool llvm::removeUnreachableBlocks(Function &F, LazyValueInfo *LVI,
 
   if (DTU) {
     DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true);
-    for (auto *BB : ToDeleteBBs)
+    bool Deleted = false;
+    for (auto *BB : ToDeleteBBs) {
+      if (DTU->isBBPendingDeletion(BB))
+        --NumRemoved;
+      else
+        Deleted = true;
       DTU->deleteBB(BB);
+    }
+    if (!Deleted)
+      return false;
   }
   return true;
 }
index 312c32e..53cfc6b 100644 (file)
@@ -844,4 +844,23 @@ TEST(Local, RemoveUnreachableBlocks) {
   runWithDomTree(*M, "br_self_loop", runLazy);
   runWithDomTree(*M, "br_constant", runLazy);
   runWithDomTree(*M, "br_loop", runLazy);
+
+  M = parseIR(C,
+              R"(
+      define void @f() {
+      entry:
+        ret void
+      bb0:
+        ret void
+      }
+        )");
+
+  auto checkRUBlocksRetVal = [&](Function &F, DominatorTree *DT) {
+    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
+    EXPECT_TRUE(removeUnreachableBlocks(F, nullptr, &DTU));
+    EXPECT_FALSE(removeUnreachableBlocks(F, nullptr, &DTU));
+    EXPECT_TRUE(DTU.getDomTree().verify());
+  };
+
+  runWithDomTree(*M, "f", checkRUBlocksRetVal);
 }
\ No newline at end of file