This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix a tcb crash and a potential bug on mainline
On Wednesday 20 October 2004 15:14, Andrew MacLeod wrote:
> 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.
Correct.
> 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...
Well, it wouldn't help because the block is made unreachable by
cleanup_control_flow().
What happens is that cleanup_control_flow() uses FOR_EACH_BB which
walks BBs from index 0 to last_basic_block, so it sees BB 2 before
BB 4. In the test case, cleaning up control flow out of BB 2 makes
BB 4 unreachable. So calling delete_unreachable_blocks() before
cleanup_control_flow() would not help in this case.
Hence my patch to make cleanup_control_flow() ignore unreachable BBs.
> 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.
That is what I believe to be the problem. Jeff thinks this is "the
right thing" to do.
> Why not allow the PHI node to exist with no arguments.
That would work too, I guess.
> > 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.
Exactly my feeling about this.
Gr.
Steven