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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I think this needs more testcases to illustrate the different possibilities here.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:737
+  Function &F = *BB->getParent();
+  DominatorTree DT(F);
   // Do not delete loop preheaders if doing so would create a critical edge.
----------------
If you really need domtree, at least hoist computing it into it out into eliminateMostlyEmptyBlocks.  (Computing it here leads to quadratic complexity for large functions.)


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:784
+       auto *DestPN = dyn_cast<PHINode>(&*DestBBI); ++DestBBI) {
+    if (HasAllUsesDominatedByPred && !AllUsesDominatedByBlock(DestPN, Pred, DT))
+      HasAllUsesDominatedByPred = false;
----------------
Is this predicate significantly different from `dominates(Pred, DestBB)`?  I mean, in theory it's slightly different, but the difference isn't relevant in your testcase.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:816
+      })) {
+    dbgs()<<DestBB->getName()<<"\n";
+    return true;
----------------
Leftover debug dump?


================
Comment at: test/Transforms/CodeGenPrepare/skip-merging-case-block.ll:26
+  %v7 = or i64 %a1, 2251799813685248
+  br i1 undef, label %b8, label %b10
+
----------------
Please try to avoid undef in testcases.


https://reviews.llvm.org/D37343