Re: Fix threading bug with computed gotos

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

On Tue, 2005-05-10 at 11:05 -0400, Diego Novillo wrote:
> The pass reorganization I've been working on exposed a bug in the
> jump threader that causes an ICE in gcc.dg/tree-ssa/20030821-1.c.
> The IL is now sufficiently cleaned up, that we ICE in DOM1
> because of this bug.
> 
> This test case reproduces the ICE in today's mainline:
> 
> -----------------------------------------------------------------------------
> void bar (int k)
> {
>   void *label = (k) ? &&x : &&y;
>   if (k)
>     goto *label;
> 
> x:
>   if (k)
>     dont_remove ();
> y:
>   return;
> }
> 
> $ ./cc1 -O1 thread-bug.c
>  bar
> 
> thread-bug.c: In function 'bar':
> thread-bug.c:2: internal compiler error: in tree_redirect_edge_and_branch, at tree-cfg.c:4549
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <URL:http://gcc.gnu.org/bugs.html> for instructions.
> -----------------------------------------------------------------------------
> 
> The problem is that we try to redirect the edges on a block that
> ends in a GOTO_EXPR:
> 
> -----------------------------------------------------------------------------
> bar (k)
> {
>   [ ... ]
>   # BLOCK 8
>   # PRED:
> <L12>:;
>   goto <bb 5> (<L6>);
>   # SUCC: 5 (fallthru)
> }
> -----------------------------------------------------------------------------
> 
> Jeff had suggested looking at create_block_for_threading because
> that's where we remove the control statements at the end of
> blocks.  However, when block 8 is created, the original block
> looks like this:
> 
> -----------------------------------------------------------------------------
> ;; basic block 8, loop depth 0, count 0
> ;; prev block 7, next block -2
> ;; pred:
> ;; succ:       5 (true,exec) 6 (false,exec)
> <bb 8>:
> if (k_2 != 0) goto <L6>; else goto y;
> -----------------------------------------------------------------------------
> 
> When we later get to redirect_edges, the block has been changed to:
> 
> -----------------------------------------------------------------------------
> ;; basic block 8, loop depth 0, count 0
> ;; prev block 7, next block -2
> ;; pred:
> ;; succ:       5 (fallthru)
> <L12>:;
> goto <bb 5> (<L6>);
> -----------------------------------------------------------------------------
> 
> I am not sure what transforms the block into this, but I also
> don't think it's doing the wrong thing.  What I did is copy the
> call to remove_ctrl_stmt_and_useless_edges from
> create_block_for_threading into redirect_edges.
> 
> My vague notion is that we also need to remove the useless
> expressions right before redirecting the edges to account for
> optimizations we may have done on the duplicate block.
> 
> Jeff, I don't really know if I am fixing or papering over the
> problem.  Is this right?
> 
> With this patch, the test now works and DOM1 converts it into:
> 
> bar (k)
> {
>   if (k_2 != 0) goto <L2>; else goto <L11>;
> 
>   # label_1 = PHI <&x(0)>;
> <L2>:;
>   goto label_1;
> 
> x:;
> <L6>:;
>   dont_remove ();
> 
> y:;
> <L9>:;
>   return;
> 
>   # label_4 = PHI <&y(0)>;
> <L11>:;
> }
> 
> 
> Bootstrap/testing in progress.
Definitely not OK.  I think you're papering over the real problem.  As
I mentioned a short while ago, I'm looking at this.
jeff