[PATCH] D33787: [IR] BasicBlock updatePHIs function for updating predecessors

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/IR/BasicBlock.cpp:449
+  bool newPredecessor = false;
+  for (BasicBlock *Pred : predecessors(this)) {
+    if (Pred == IncomingOld)
----------------
I'm not sure this loop is necessary.
If this funciton is how to update phi edges, you should guarantee an input state.
IE either pred edges removed or not removed.




================
Comment at: lib/IR/BasicBlock.cpp:457
+  if (!newPredecessor) {
+    errs() << "WARNING: attempting to update PHI predecessor for block with no "
+              "such predecessor\n";
----------------
This should be an assert


================
Comment at: lib/IR/BasicBlock.cpp:466
+      if (oldPredecessor) {
+        // Cannot just change the incoming when IncomingOld is still valid,
+        // need to insert another value with the same incoming value.
----------------
Can you explain why?
This seems like  a bug?


https://reviews.llvm.org/D33787