[PATCH] D14308: [SimplifyCFG] Extend SimplifyResume to handle phi of trivial landing pad.

[Chen Li via llvm-commits]

chenli added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3242
@@ +3241,3 @@
+    // Check incoming blocks to see if any of them are trivial.
+    for (unsigned i = 0; i < PhiLPInst->getNumIncomingValues(); i++) {
+      auto *IncomingBB = PhiLPInst->getIncomingBlock(i);
----------------
majnemer wrote:
> Please do not evaluate `getNumIncomingValues` each time through the loop, consider writing this like:
>   for (unsigned Idx = 0, End = PhiLPInst->getNumIncomingValues(); Idx != End; ++Idx) {
Will do. But I think the compiler should be able to optimize this.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3295
@@ +3294,3 @@
+  // haven't already deleted it above when deleting the landing pad blocks.
+  if (BB && pred_begin(BB) == pred_end(BB)) {
+    BB->eraseFromParent();
----------------
majnemer wrote:
> Isn't this `BB && pred_empty(BB)` ?  Also, the braces are superfluous.
Will do.

================
Comment at: test/Transforms/SimplifyCFG/bug-25299.ll:1
@@ +1,2 @@
+; RUN: opt < %s -simplifycfg -disable-output
+
----------------
majnemer wrote:
> No FileCheck?
The only purpose of this test is to verify there's no crash. The other test should be able to catch miscompiles. But I can definitely add FileCheck if it's useful in general. 


http://reviews.llvm.org/D14308