[PATCH] D49738: [Dominators] Make RemoveUnreachableBlocks return false if the BasicBlock is already awaiting deletion

[Chijun Sima via Phabricator via llvm-commits]

NutshellySima created this revision.
NutshellySima added reviewers: kuhar, brzycki, dmgreen, grosser, davide.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D49738

Files:
  lib/Transforms/Utils/Local.cpp
  unittests/Transforms/Utils/Local.cpp


Index: unittests/Transforms/Utils/Local.cpp
===================================================================
--- unittests/Transforms/Utils/Local.cpp
+++ unittests/Transforms/Utils/Local.cpp
@@ -618,3 +618,26 @@
   verifyModule(*M, &errs(), &BrokenDebugInfo);
   ASSERT_FALSE(BrokenDebugInfo);
 }
+
+TEST(Local, RemoveUnreachableBlocks) {
+  LLVMContext C;
+
+  std::unique_ptr<Module> M = parseIR(C,
+                                      R"(
+      define void @f() {
+      entry:
+        br label %entry
+      bb0:
+        ret void
+      }
+        )");
+
+  auto callRemoveUnreachableBlocks = [&](Function &F, DominatorTree *DT) {
+    DeferredDominance DDT(*DT);
+    EXPECT_TRUE(removeUnreachableBlocks(F, nullptr, &DDT));
+    EXPECT_FALSE(removeUnreachableBlocks(F, nullptr, &DDT));
+    EXPECT_TRUE(DDT.flush().verify());
+  };
+
+  runWithDomTree(*M, "f", callRemoveUnreachableBlocks);
+}
Index: lib/Transforms/Utils/Local.cpp
===================================================================
--- lib/Transforms/Utils/Local.cpp
+++ lib/Transforms/Utils/Local.cpp
@@ -2233,13 +2233,18 @@
     BB->dropAllReferences();
   }
 
+  bool Deleted = false;
   for (Function::iterator I = ++F.begin(); I != F.end();) {
     auto *BB = &*I;
     if (Reachable.count(BB)) {
       ++I;
       continue;
     }
     if (DDT) {
+      if (DDT->pendingDeletedBB(BB)) // BB has already been deleted.
+        --NumRemoved;
+      else
+        Deleted = true;
       DDT->deleteBB(BB); // deferred deletion of BB.
       ++I;
     } else {
@@ -2249,6 +2254,8 @@
 
   if (DDT)
     DDT->applyUpdates(Updates);
+  if (DDT && !Deleted)
+    return false;
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49738.157045.patch
Type: text/x-patch
Size: 1678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180724/c9d49758/attachment.bin>