[PATCH] D37343: [CGP] Merge empty case blocks if no extra moves are added.

[Balaram Makam via Phabricator via llvm-commits]

bmakam added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:720
+      DT->changeImmediateDominator(DestBB, NewIDom);
+      DT->eraseNode(BB);
+    }
----------------
bmakam wrote:
> efriedma wrote:
> > New code should be using insertEdge/deleteEdge/applyUpdates to do domtree updates.
> I found this to be more easy to understand, I am happy to change this to DominatorTree::UpdateTypes/applyUpdates if this is the new coding standard.
I tried using the new batch updater interface for DominatorTree. I create a vector of updates. First, insert edges from all predecessors of BB to DestBB. Second, remove edges from all predecessors of BB to BB  and finally, delete edge from BB to DestBB. IIUC, the batch updater is efficient for multiple CFG updates, but in this case updating the IDom and erasing the Node seems more efficient to me.

```
+    eliminateMostlyEmptyBlock(BB);
     if (DT && !ModifiedDT) {
+/*
       BasicBlock *BBIDom = DT->getNode(BB)->getIDom()->getBlock();
       BasicBlock *DestBBIDom = DT->getNode(DestBB)->getIDom()->getBlock();
       BasicBlock *NewIDom = DT->findNearestCommonDominator(BBIDom, DestBBIDom);
       DT->changeImmediateDominator(DestBB, NewIDom);
       DT->eraseNode(BB);
+*/
+      SmallVector<DominatorTree::UpdateType, 3> Updates;
+      for (auto *PredBB : predecessors(BB)) {
+        if (PredBB == BB)
+          continue;
+        DominatorTree::UpdateType UT = {DominatorTree::Insert, PredBB, DestBB};
+        if (find(Updates, UT) == Updates.end()) {
+          Updates.push_back(UT);
+          Updates.push_back({DominatorTree::Delete, PredBB, BB});
+        }
+      }
+      if (!Updates.empty()) {
+        Updates.push_back({DominatorTree::Delete, BB, DestBB});
+        DT->applyUpdates(Updates);
+      }
     }
-    eliminateMostlyEmptyBlock(BB);
```
What do you think?


https://reviews.llvm.org/D37343