[PATCH] Fixup PHI nodes in LowerSwitch

[Philip Reames]

I spotted another issue on this read through, but generally looking good.  Once you've made this change, ping me directly and I'll give it a final read through before giving a LGTM.

================
Comment at: lib/Transforms/Utils/LowerSwitch.cpp:138
@@ +137,3 @@
+ for (BasicBlock::iterator I = Succ->begin(),
+                           E = Succ->getFirstNonPHIOrDbgOrLifetime();
+      I != E; ++I) {
----------------
I know this was discussed in another review, but I'm pretty sure this is not necessary.  I have no objection to you using this construct - i.e. go ahead and submit - but it would be good to clarify on llvm-dev and revisit if need be.  If this construct is actually required, I have quite a bit of out of tree code to fix.  :)

================
Comment at: lib/Transforms/Utils/LowerSwitch.cpp:145
@@ +144,3 @@
+
+   int BlockIdx = PN->getBasicBlockIndex(OrigBlock);
+   assert(BlockIdx != -1 && "Switch didn't go to this successor??");
----------------
Oh, I just noticed this.  A single basic block can reach a successor with different values.  (e.g. consider a switch which simply forwards the case value, before that's optimized away) 

The getBasicBlockIndex will return the first such edge, not all edges.  You probably want to write:
for each incoming edge: update if bb == predecessor

It would be good to add a test case for this if possible.  If you believe this is not possible, add a comment and assert that checks that.  (i.e. after the update, no remaining incoming edge from OrigBlock.)



================
Comment at: test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll:2
@@ +1,3 @@
+; RUN: opt < %s -lowerswitch -S | FileCheck %s
+; CHECK: phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
+; CHECK: phi i32 [ 0, %NewDefault ]
----------------
Please add a check-label or otherwise give context here.  Where are these phis expected? What should be around them?

Also, could you place them in the body of the test function?  That way, we can add more tests to this file.  You'll need to add a CHECK: test

http://reviews.llvm.org/D4298