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

[Mark Lacey]

Hi Duncan,

On Jun 6, 2013, at 11:26 AM, Duncan Sands <duncan.sands at gmail.com> wrote:
> Hi Mark,
> 
> On 06/06/13 19:27, Mark Lacey wrote:
>> 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).
> 
> thanks for the detailed explanation.  However I don't understand how you handle
> the case where PrevBB is already present multiple times in PN.  I think you
> only update the first instance of PrevBB, not all instances.
> 

Yes, you're quite right. In the case where the block being merged into had a phi with multiple undef inputs from the common predecessor, only some of them will be updated.

I have attached a new patch that addresses this, and also restructures my changes to make them a bit more straightforward and clear.

Thanks again for your time and feedback,

Mark

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-blocks-to-be-merged-when-one-has-an-undef-inpu.patch
Type: application/octet-stream
Size: 16809 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130606/37382e96/attachment.obj>