Re: [PATCH] Improve debug info if tree DCE removes stores (PR debug/50317, fallout)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Dec 2, 2011 at 8:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Dec 02, 2011 at 02:27:40PM +0100, Richard Guenther wrote:
>> This change seems wrong.  We are turning valid gimple
>>
>> # DEBUG D#2 => transfer.0  [with addres taken]
>>
>> into invalid one
>>
>> # DEBUG D#2 => transfer.0  [without address taken]
>>
>> once you update that stmt with update_stmt you'll get an SSA operand
>> for transfer.0 which is not in SSA form because you fail to rewrite it
>> into.
>>
>> Why do this in remove_unused_locals and not in update_address_taken?
>> Or, why do it at all?
>>
>> I have a SSA operand checking patch that catches this now ...
>
> Here is a fix for that.  Instead of clearing TREE_ADDRESSABLE for
> unreferenced vars we allow them in target_for_debug_bind if they aren't
> referenced vars (thus we don't risk mixing VALUE tracking with the
> old style REG_EXPR/MEM_EXPR tracking of these variables).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2011-12-02  Jakub Jelinek  <jakub@redhat.com>
>
>         PR debug/50317
>         * tree-ssa.c (target_for_debug_bind): Also allow is_gimple_reg_type
>         vars that aren't referenced.
>         (tree-ssa-live.c (remove_unused_locals): Don't clear TREE_ADDRESSABLE
>         of unreferenced local vars.
>         * cfgexpand.c (expand_debug_expr): For DEBUG_IMPLICIT_PTR allow also
>         TREE_ADDRESSABLE vars that satisfy target_for_debug_bind.
>
> --- gcc/tree-ssa.c.jj   2011-11-29 08:58:52.000000000 +0100
> +++ gcc/tree-ssa.c      2011-12-02 15:04:03.494148642 +0100
> @@ -264,7 +264,12 @@ target_for_debug_bind (tree var)
>      return NULL_TREE;
>
>    if (!is_gimple_reg (var))
> -    return NULL_TREE;
> +    {
> +      if (is_gimple_reg_type (TREE_TYPE (var))
> +         && referenced_var_lookup (cfun, DECL_UID (var)) == NULL_TREE)
> +       return var;
> +      return NULL_TREE;
> +    }

With referenced-vars going away now I am running into the above use of
referenced-vars again.  So the above tries to increase the set of tracked
"registers" by noting that unused non-registers that might have become
registers are supposed to be tracked anyway.  But we do _not_ want to
have used non-register (but register type) variables tracked because we
do not track aliases?  Or because that would be prohibitively expensive?

That is, why is

Index: tree-ssa.c
===================================================================
--- tree-ssa.c  (revision 190031)
+++ tree-ssa.c  (working copy)
@@ -259,13 +259,9 @@ target_for_debug_bind (tree var)
   if (DECL_IGNORED_P (var))
     return NULL_TREE;

-  if (!is_gimple_reg (var))
-    {
-      if (is_gimple_reg_type (TREE_TYPE (var))
-         && referenced_var_lookup (cfun, DECL_UID (var)) == NULL_TREE)
-       return var;
-      return NULL_TREE;
-    }
+  /* var-tracking only tracks registers.  */
+  if (!is_gimple_reg_type (TREE_TYPE (var)))
+    return NULL_TREE;

   return var;
 }

not what we want (minus dealing with fallout of having mixed SSA / non-SSA
after something makes var suitable for renaming into SSA form)?

Richard.

>    return var;
>  }
> --- gcc/tree-ssa-live.c.jj      2011-12-02 01:52:27.000000000 +0100
> +++ gcc/tree-ssa-live.c 2011-12-02 15:04:59.601816335 +0100
> @@ -814,15 +814,7 @@ remove_unused_locals (void)
>               bitmap_set_bit (global_unused_vars, DECL_UID (var));
>             }
>           else
> -           {
> -             /* For unreferenced local vars drop TREE_ADDRESSABLE
> -                bit in case it is referenced from debug stmts.  */
> -             if (DECL_CONTEXT (var) == current_function_decl
> -                 && TREE_ADDRESSABLE (var)
> -                 && is_gimple_reg_type (TREE_TYPE (var)))
> -               TREE_ADDRESSABLE (var) = 0;
> -             continue;
> -           }
> +           continue;
>         }
>        else if (TREE_CODE (var) == VAR_DECL
>                && DECL_HARD_REGISTER (var)
> --- gcc/cfgexpand.c.jj  2011-12-02 01:52:27.000000000 +0100
> +++ gcc/cfgexpand.c     2011-12-02 15:24:37.982327507 +0100
> @@ -3325,7 +3325,8 @@ expand_debug_expr (tree exp)
>           if ((TREE_CODE (TREE_OPERAND (exp, 0)) == VAR_DECL
>                || TREE_CODE (TREE_OPERAND (exp, 0)) == PARM_DECL
>                || TREE_CODE (TREE_OPERAND (exp, 0)) == RESULT_DECL)
> -             && !TREE_ADDRESSABLE (TREE_OPERAND (exp, 0)))
> +             && (!TREE_ADDRESSABLE (TREE_OPERAND (exp, 0))
> +                 || target_for_debug_bind (TREE_OPERAND (exp, 0))))
>             return gen_rtx_DEBUG_IMPLICIT_PTR (mode, TREE_OPERAND (exp, 0));
>
>           if (handled_component_p (TREE_OPERAND (exp, 0)))
> @@ -3337,7 +3338,8 @@ expand_debug_expr (tree exp)
>               if ((TREE_CODE (decl) == VAR_DECL
>                    || TREE_CODE (decl) == PARM_DECL
>                    || TREE_CODE (decl) == RESULT_DECL)
> -                 && !TREE_ADDRESSABLE (decl)
> +                 && (!TREE_ADDRESSABLE (decl)
> +                     || target_for_debug_bind (decl))
>                   && (bitoffset % BITS_PER_UNIT) == 0
>                   && bitsize > 0
>                   && bitsize == maxsize)
>
>
>         Jakub