<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; ">Hi Nick,<div><br></div><div>Thanks for the review.</div><div><br><div><div>On Jun 5, 2013, at 5:27 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr">On 5 June 2013 16:24, Mark Lacey <span dir="ltr"><<a href="mailto:mark.lacey@apple.com" target="_blank">mark.lacey@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Can someone please review, and commit if everything looks good?<br></blockquote><div><br></div><div>This file is overwhelmingly star-on-the right style. Please use "Value *" in place of "Value* " for consistency.</div></div></div></div></blockquote><div><br></div><div>Fixed in the attached update.</div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div style="">+ assert(CanMergeValues(First, Second));</div>
<div style=""><br></div><div style="">Please add a " && message" to this assert.</div></div></div></div></blockquote><div><br></div>Added a message here, too.</div><div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div style=""><br></div><div style=""><div>--- /dev/null</div><div>+++ b/test/Transforms/SimplifyCFG/UndefPHIEdgeBlockMerge.ll</div>
<div><br></div><div style="">Please merge your test in with 2008-05-16-PHIBlockMerge.ll (and remove the date from its name, date-named tests are deprecated). Possibly also merge 2009-01-18-PHIPropCrash.ll in as well.</div></div></div></div></div></blockquote><br></div><div>I moved my tests to EqualPHIEdgeBlockMerge.ll, and merged 2008-05-16-PHIBlockMerge.ll in as well. It looks to me like 2009-01-18-PHIPropCrash.ll is not testing the same merging optimization, so I've left that as a separate file for now. Let me know if you think it makes more sense to move those tests into EqualPHIEdgeBlockMerge.ll as well. </div><div><br></div><div>Otherwise, I think I have addressed all of your comments with the new patch below.</div><div><br></div><div>Thanks,</div><br></div><div>Mark</div><div><br></div><div></div></body></html>