[PATCH] Allow blocks to be merged when one has an undef input to a phi.

[Mark Lacey]

Hi Duncan,

On Jun 6, 2013, at 7:21 AM, Duncan Sands <duncan.sands at gmail.com> wrote:
>> All of the predecessors of BB must be added to the phi node since we rewrite all of the uses of BB (in terminators) to target Succ. Otherwise we would have a conditional branch or switch that branches to Succ on multiple edges (e.g. both sides of the conditional branch, or multiple cases of the switch), but not have as many incoming edges in the phi.
> 
> if (PredBBIdx >= 0) was true, then PN already has PredBB has a predecessor.
> Indeed, you just assigned a value to it in the line
>  PN->setIncomingValue(PredBBIdx, PredVal);
> But then you fall through to the line
>  PN->addIncoming(PredVal, PredBB);
> and add a second copy of PredBB and PredVal to PN.  Isn't this wrong?

We need to add the incoming values to the phi for all the new edges being redirected from the predecessor (this was already happening before my change as well). 

Consider a case like this:

define i8 @testmergesome(i32 %u, i32* %A) {
V:
  switch i32 %u, label %Y [
    i32 0, label %W
    i32 1, label %X
    i32 2, label %X
    i32 3, label %Z
  ]

W:                                            ; preds = %V
  store i32 1, i32* %A, align 4
  br label %Y

Z:
  store i32 0, i32* %A, align 4
  br label %X

X:                                           ; preds = %V, %Z
  br label %Y

Y:                                        ; preds = %X, %W, %V
  ; After merging X into Y, we should have 5 predecessors
  ; and thus 5 incoming values to the phi.
  %val.0 = phi i8 [ 1, %V ], [ 1, %X ], [ 2, %W ]
  ret i8 %val.0
}

When we merge X and Y, we need to add X's predecessors (V and Z) with incoming values to the phi, despite the fact that the phi already as an incoming edge from V (and despite the fact that the value coming in is the same). 

Otherwise we would end up with a result like this:

define i8 @testmergesome(i32 %u, i32* %A) {
V:
  switch i32 %u, label %Y [
    i32 0, label %W
    i32 1, label %Y
    i32 2, label %Y
    i32 3, label %Z
  ]

W:                                                ; preds = %V
  store i32 1, i32* %A, align 4
  br label %Y

Z:                                                ; preds = %V
  store i32 0, i32* %A, align 4
  br label %Y

Y:                                                ; preds = %V, %V, %Z, %W, %V
  %val.0 = phi i8 [ 1, %V ], [ 2, %W ], [ 1, %Z ]
  ret i8 %val.0
}


Here, Y has five predecessors but only three incoming values in the phi.

These additional incoming values and associated control-flow do end up getting cleaned up properly in other simplifications  (e.g. in this instance case 1 and 2 of the switch would be removed by switch simplification since they now target the same block as the switch default).

Mark