Re: Fix a tcb crash and a potential bug on mainline

[Andrew MacLeod <amacleod@redhat.com>]

On Tue, 2004-10-19 at 19:13, Jeffrey A Law wrote:
> On Tue, 2004-10-19 at 16:36, Steven Bosscher wrote:
> 
> > > Err, if we remove the last argument to a PHI node then by definition
> > > the block containing the PHI is unreachable as it has no incoming
> > > edges.
> > >
> > > There is no such thing as an "undefined" PHI node.
> > 
> > I didn't say we do.  I said we may have an undefined PHI result,
> > i.e. and SSA_NAME without a definition.
> OK.  And that is something that is, unfortunately, normal in our
> implementation.  If I look at the SSA_NAME manager that's by far
> its largest wart.
> 

If I understand the problem properly, the fundamental problem here is
that a block becomes unreachable by removing all the incoming edges. PHI
arguments for those edges are removed as the edge is removed. When the
last edge is removed, the PHI node has no more arguments, and thus gets
deleted.  None of the code in the dead block is removed until later when
delete_unreachable_blocks is called. Meanwhile, we end up pawing through
one or more stmts in the dead block which contains uses of the PHI
result.

I have a couple of questions.

1 - Why does cleanup_control_flow() ever look at a stmts inside an
unreachable block? Its seems rather, umm, unnecessary, for starters. Why
not just call delete_unreachable_blocks() as the *first* thing in
cfg_cleanup(). It doesn't look like its an expensive call... 

2 - The issue may also exist outside cleanup_control_flow, so why does
remove_phi_arg_num have to remove the PHI node when is removes the last
argument. Why not allow the PHI node to exist with no arguments. It'll
get removed and free'd if the block remains unreachable. Maybe someone
will add another edge back in at some point during the optimization,
then there are PHI nodes ready to take the arguments.  I presume right
now we make sure we add new edges before deleting old ones when
redirecting things (to avoid deleting the PHI).  This might cause the
PHI node to be resized larger without really needing it. (ie, if you are
replacing two edges with one different one, and you add the one before
delete the other two, you've resized the PHI node to 3.)
 

> 
>   2. When we release an SSA_NAME and PHI_NODE go and find all uses
>      of the SSA_NAME and release them too.  This is effectively an
>      iterative DCE process.

I am not very fond of this one :-)  Especially since they are going to
be dealt with en-mass when you delete the block anyway.

Andrew