Re: [patch] tree-phinode.c: Clean up remove_phi_node.

[Jeffrey A Law <law at redhat dot com>]

On Wed, 2005-03-02 at 16:22 -0500, Kazu Hirata wrote:
> Hi,
> 
> Attached is a patch to clean up remove_phi_node.
> 
> First note that there is some code duplication, namely calls to
> release_ssa_name and release_phi_node.
> 
> Next note that the first iteration of the "for" loop in
> remove_phi_node never has "t == phi" because of the second "if"
> statement.
> 
> The patch cleans up remove_phi_node while resolving the problems
> above.  It assumes that bb_for_stmt (phi) == bb.
> 
> Looking at the tree dump, I see that remove_phi_node now has 5 basic
> blocks instead of 15 basic blocks.
> 
> The rdtsc instruction on x86 shows 5% speed-up although its effect is
> not measurable on a larger scale.
> 
> Tested on i686-pc-linux-gnu.  OK to apply?
> 
> Kazu Hirata
> 
> 2005-03-02  Kazu Hirata  <kazu@cs.umass.edu>
> 
> 	* tree-phinodes.c (remove_phi_node): Clean up by factoring out
> 	calls to release_ssa_name and release_phi_node.
This is fine.

I will note the behavior of the new code is not exactly the same as
the old code.

Specifically if the old code would do nothing if the passed in PHI
was not in the chain of PHI nodes attached to BB.  I'm pretty sure
the new code will segfault.

You might consider a follow-up patch which gives a more graceful
failure mode via gcc_assert rather than segfaulting :-)

Thanks,
Jeff