<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div></div><div><br></div><div><br></div><br><div><div>On Jun 8, 2013, at 1:38 PM, Mark Lacey <<a href="mailto:mark.lacey@apple.com">mark.lacey@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi Duncan,<br><br>I just realized I re-attached the old patch when I sent this. I've now attached the correct patch.<br><br>Thanks,<br><br>Mark<br><br><span><0001-Allow-blocks-to-be-merged-when-one-has-an-undef-inpu.patch></span><br><br>On Jun 6, 2013, at 7:15 PM, Mark Lacey <<a href="mailto:mark.lacey@apple.com">mark.lacey@apple.com</a>> wrote:<br><br><blockquote type="cite">Hi Duncan,<br><br>On Jun 6, 2013, at 11:26 AM, Duncan Sands <<a href="mailto:duncan.sands@gmail.com">duncan.sands@gmail.com</a>> wrote:<br><blockquote type="cite">Hi Mark,<br><br>On 06/06/13 19:27, Mark Lacey wrote:<br><blockquote type="cite">Hi Duncan,<br><br>On Jun 6, 2013, at 7:21 AM, Duncan Sands <<a href="mailto:duncan.sands@gmail.com">duncan.sands@gmail.com</a>> wrote:<br><blockquote type="cite"><blockquote type="cite">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.<br></blockquote><br>if (PredBBIdx >= 0) was true, then PN already has PredBB has a predecessor.<br>Indeed, you just assigned a value to it in the line<br>PN->setIncomingValue(PredBBIdx, PredVal);<br>But then you fall through to the line<br>PN->addIncoming(PredVal, PredBB);<br>and add a second copy of PredBB and PredVal to PN. Isn't this wrong?<br></blockquote><br>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).<br></blockquote><br>thanks for the detailed explanation. However I don't understand how you handle<br>the case where PrevBB is already present multiple times in PN. I think you<br>only update the first instance of PrevBB, not all instances.<br><br></blockquote><br>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.<br><br>I have attached a new patch that addresses this, and also restructures my changes to make them a bit more straightforward and clear.<br><br>Thanks again for your time and feedback,<br><br>Mark<br><br><0001-Allow-blocks-to-be-merged-when-one-has-an-undef-inpu.patch><br></blockquote><br></blockquote></div><br></body></html>