Re: [PATCH] Fix tree-into-ssa.c for debug stmts (PR debug/49602)

[Richard Guenther <richard.guenther@gmail.com>]

On Fri, Jul 1, 2011 at 11:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> For debug stmt uses, we don't want any PHI nodes to be created
> just because of them, so the debug uses need to be ignored
> during decisions which PHI nodes to add.
> Unfortunately that means get_current_def can't be always trusted.
> The following patch trusts it in a few cases where I'm reasonably
> sure it is trustable.  Perhaps some more cases could be added in
> the future if anyone has ideas...
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-07-01  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/49602
>        * tree-into-ssa.c (rewrite_debug_stmt_uses): Disregard
>        get_current_def return value if it can't be trusted to be
>        the current value of the variable in the current bb.
>
>        * gcc.dg/pr49602.c: New test.
>
> --- gcc/tree-into-ssa.c.jj      2011-06-23 10:13:58.000000000 +0200
> +++ gcc/tree-into-ssa.c 2011-07-01 20:39:40.000000000 +0200
> @@ -1343,7 +1343,41 @@ rewrite_debug_stmt_uses (gimple stmt)
>            }
>        }
>       else
> -       def = get_current_def (var);
> +       {
> +         def = get_current_def (var);
> +         /* Check if get_current_def can be trusted.  */
> +         if (def)
> +           {
> +             basic_block bb = gimple_bb (stmt);
> +             basic_block def_bb
> +               = SSA_NAME_IS_DEFAULT_DEF (def)
> +                 ? NULL : gimple_bb (SSA_NAME_DEF_STMT (def));
> +
> +             /* If definition is in current bb, it is fine.  */
> +             if (bb == def_bb)
> +               ;
> +             /* If definition bb doesn't dominate the current bb,
> +                it can't be used.  */
> +             else if (def_bb && !dominated_by_p (CDI_DOMINATORS, bb, def_bb))
> +               def = NULL;
> +             /* If there is just one definition and dominates the current
> +                bb, it is fine.  */
> +             else if (get_phi_state (var) == NEED_PHI_STATE_NO)
> +               ;
> +             else
> +               {
> +                 struct def_blocks_d *db_p = get_def_blocks_for (var);
> +
> +                 /* If there are some non-debug uses in the current bb,
> +                    it is fine.  */
> +                 if (bitmap_bit_p (db_p->livein_blocks, bb->index))
> +                   ;
> +                 /* Otherwise give up for now.  */
> +                 else
> +                   def = NULL;
> +               }
> +           }
> +       }
>       if (def == NULL)
>        {
>          gimple_debug_bind_reset_value (stmt);
> --- gcc/testsuite/gcc.dg/pr49602.c.jj   2011-07-01 20:42:44.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr49602.c      2011-07-01 18:49:04.000000000 +0200
> @@ -0,0 +1,17 @@
> +/* PR debug/49602 */
> +/* { dg-do compile } */
> +/* { dg-options "-g -O2" } */
> +
> +static void
> +foo (int *x)
> +{
> +}
> +
> +void
> +bar (int *x)
> +{
> +  int i;
> +  for (i = 0; i == 1; ++i)
> +    x = 0;
> +  foo (x);
> +}
>
>        Jakub
>