[llvm] r338882 - [Dominators] Make RemoveUnreachableBlocks return false if the BasicBlock is already awaiting deletion

[Chijun Sima via llvm-commits]

Author: sima
Date: Fri Aug  3 05:45:29 2018
New Revision: 338882

URL: http://llvm.org/viewvc/llvm-project?rev=338882&view=rev
Log:
[Dominators] Make RemoveUnreachableBlocks return false if the BasicBlock is already awaiting deletion

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

Modified:
    llvm/trunk/lib/Transforms/Utils/Local.cpp
    llvm/trunk/unittests/Transforms/Utils/Local.cpp

Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=338882&r1=338881&r2=338882&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Fri Aug  3 05:45:29 2018
@@ -2264,8 +2264,16 @@ bool llvm::removeUnreachableBlocks(Funct
 
   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;
 }

Modified: llvm/trunk/unittests/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Local.cpp?rev=338882&r1=338881&r2=338882&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/Local.cpp Fri Aug  3 05:45:29 2018
@@ -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