[PATCH] D48256: Fix bug to merge away entry block and update DT correctly.

[Xin Tong via Phabricator via llvm-commits]

trentxintong updated this revision to Diff 151627.
trentxintong added a comment.

Remove some duplicate code.


Repository:
  rL LLVM

https://reviews.llvm.org/D48256

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
@@ -176,6 +176,18 @@
 TEST(Local, MergeBasicBlockIntoOnlyPred) {
   LLVMContext C;
 
+  auto runMergeBasicBlockIntoOnlyPred = [&](Function &F, DominatorTree *DT) {
+    for (Function::iterator I = F.begin(), E = F.end(); I != E;) {
+      BasicBlock *BB = &*I++;
+      BasicBlock *SinglePred = BB->getSinglePredecessor();
+      if (!SinglePred || SinglePred == BB || BB->hasAddressTaken())
+        continue;
+      BranchInst *Term = dyn_cast<BranchInst>(SinglePred->getTerminator());
+      if (Term && !Term->isConditional())
+        MergeBasicBlockIntoOnlyPred(BB, DT);
+    }
+  };
+
   std::unique_ptr<Module> M = parseIR(C,
                                       R"(
       define i32 @f(i8* %str) {
@@ -194,18 +206,28 @@
         ret i32 0
       }
       )");
-  runWithDomTree(
-      *M, "f", [&](Function &F, DominatorTree *DT) {
-        for (Function::iterator I = F.begin(), E = F.end(); I != E;) {
-          BasicBlock *BB = &*I++;
-          BasicBlock *SinglePred = BB->getSinglePredecessor();
-          if (!SinglePred || SinglePred == BB || BB->hasAddressTaken()) continue;
-          BranchInst *Term = dyn_cast<BranchInst>(SinglePred->getTerminator());
-          if (Term && !Term->isConditional())
-            MergeBasicBlockIntoOnlyPred(BB, DT);
-        }
-        EXPECT_TRUE(DT->verify());
-      });
+  runWithDomTree(*M, "f", [&](Function &F, DominatorTree *DT) {
+    runMergeBasicBlockIntoOnlyPred(F, DT);
+    EXPECT_TRUE(DT->verify());
+  });
+
+  // Test we can merge away entry block and update DT correctly.
+  // entry block does not have IDom.
+  std::unique_ptr<Module> M2 = parseIR(C,
+                                       R"(
+      define i32 @f(i8* %str) {
+      entry:
+        br label %bb2.i
+      bb2.i:                                            ; preds = %bb4.i, %entry
+        br label %base2flt.exit204
+      base2flt.exit204:                                 ; preds = %bb7.i197, %bb7.i197, %bb2.i, %bb4.i
+        ret i32 0
+      }
+      )");
+  runWithDomTree(*M2, "f", [&](Function &F, DominatorTree *DT) {
+    runMergeBasicBlockIntoOnlyPred(F, DT);
+    EXPECT_TRUE(DT->verify());
+  });
 }
 
 TEST(Local, ConstantFoldTerminator) {
Index: lib/Transforms/Utils/Local.cpp
===================================================================
--- lib/Transforms/Utils/Local.cpp
+++ lib/Transforms/Utils/Local.cpp
@@ -714,7 +714,8 @@
   if (ReplaceEntryBB)
     DestBB->moveAfter(PredBB);
 
-  if (DT) {
+  // Entry block does not have IDom.
+  if (DT && !ReplaceEntryBB) {
     // For some irreducible CFG we end up having forward-unreachable blocks
     // so check if getNode returns a valid node before updating the domtree.
     if (DomTreeNode *DTN = DT->getNode(PredBB)) {
@@ -735,6 +736,11 @@
       DDT->applyUpdates(Updates);
   } else {
     PredBB->eraseFromParent(); // Nuke BB.
+    // The entry block was removed and there is no external interface for the
+    // dominator tree to be notified of this change. In this corner-case we
+    // recalculate the entire tree.
+    if (DT && ReplaceEntryBB)
+      DT->recalculate(*(DestBB->getParent()));
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48256.151627.patch
Type: text/x-patch
Size: 3333 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180616/62eb9290/attachment.bin>