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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

That just m



================
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.
----------------
Marandil wrote:
> dberlin wrote:
> > Can you explain why?
> > This seems like  a bug?
> Consider this:
> 
>   switch (a) {
>     default: BB0;
>     case 0: BB0;
>     case 1: BB1;
>     case 2: BB2;
>     case 19: BB0;
>     case 20: BB1;
>     case 21: BB2;
>   }
> 
> changed into:
> 
>   if (0 <= a <=2) {
>     switch (a) {
>       default: unreachable;
>       case 0: BB0;
>       case 1: BB1;
>       case 2: BB2;
>     }
>   } else {
>     switch (a-19) {
>       default: BB0;
>       case 0: BB0;
>       case 1: BB1;
>       case 2: BB2;
>     }
>   }
> 
> For BB0 a single predecessor changed into 2 different possible predecessors, that should share the same incoming value.
That just makes me believe the API specification for this function is confusing.
Either your goal is to update all phis from one block to another, or it isn't.
In the above, you aren't.





https://reviews.llvm.org/D33787