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

Mark Lacey mark.lacey at apple.com
Wed Jun 5 21:16:43 PDT 2013


Hi Nick,

Thanks for the review.

On Jun 5, 2013, at 5:27 PM, Nick Lewycky <nlewycky at google.com> wrote:
> On 5 June 2013 16:24, Mark Lacey <mark.lacey at apple.com> wrote:
> Can someone please review, and commit if everything looks good?
> 
> This file is overwhelmingly star-on-the right style. Please use "Value *" in place of "Value* " for consistency.

Fixed in the attached update.

> +  assert(CanMergeValues(First, Second));
> 
> Please add a " && message" to this assert.

Added a message here, too.

> 
> --- /dev/null
> +++ b/test/Transforms/SimplifyCFG/UndefPHIEdgeBlockMerge.ll
> 
> 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.

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. 

Otherwise, I think I have addressed all of your comments with the new patch below.

Thanks,

Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/0cb85167/attachment.html>
-------------- 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: 15170 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/0cb85167/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/0cb85167/attachment-0001.html>


More information about the llvm-commits mailing list