[llvm-commits] Patch to delete PHI nodes with multiple uses

[Andrew Clinton]

Thank you for the review - attached is the revision.

If you could commit this to the baseline that would be appreciated!  I
have a few more patches that I'm planning to submit in the next week.

Andrew

On Sat, Feb 19, 2011 at 8:44 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Andrew Clinton wrote:
>>
>> This is a simple patch to improve RecursivelyDeleteDeadPHINode so that
>> it will delete PHIs that have multiple identical uses (self
>> references).
>
> Cool! I have some review comments:
>
> +// Check whether the uses of a value are all the same.  This is similar to
> +// Instruction::hasOneUse() except this will also return true when there
> are
> +// multiple uses that all refer to the same value.
> +static bool
> +AreAllUsesEqual(Instruction *J)
> +{
>
> The previous three lines all fit on one. Also, why "J"? "I" isn't used in
> this function. Also, since this is a new function, please start the name
> with an initial lowercase letter
> (http://llvm.org/docs/CodingStandards.html#ll_naming is newer than most of
> the code, in case you're wondering).
>
> +  Value::use_iterator UI = J->use_begin();
> +  Value::use_iterator UE = J->use_end();
> +  if (UI == UE)
> +    return false;
> +
> +  for (++UI; UI != UE; ++UI)
> +  {
> +    if (*UI != *J->use_begin())
>
> Could you hoist the computation of *J->use_begin() out of the loop?
>
> +      return false;
> +  }
> +  return true;
> +}
>
>> The DeletePHIs.cpp is a pass to test it, along with delete-phis.ll.
>> I'm not sure whether this test needs to be added to the baseline or if
>> so, where to put it.  Run the test with:
>
> The delete-phis.ll test is convincing enough, I don't think this warrants
> keeping an entire pass -- but if you do want to run this method over some
> code, I suggest writing a unit test for it under unittests/. You can produce
> a Function then call RecursivelyDeleteDeadPHINode on an instruction and
> check the result.
>
> Please post an updated patch. Do you want me to commit this for you?
>
> Nick
>
>> opt -load ../../../Debug+Asserts/lib/LLVMDeletePHIs.so<
>> delete-phis.ll -delete-phis -S
>>
>> Andrew
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: deletephis.diff
Type: text/x-patch
Size: 3366 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110219/1debc419/attachment.bin>