diff mbox

Improve debug info if tree DCE removes stores (PR debug/50317, fallout)

Message ID CAFiYyc3wMv4Xw9GFQrN-GgJXZQ6aywVt123f5-jF+J339gJ27g@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Aug. 1, 2012, 10:03 a.m. UTC 
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


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

Comments

Alexandre Oliva Sept. 5, 2012, 10:33 p.m. UTC | #1 
Hi, Richi,

Sorry if this comes in late.  I'd saved your message for careful
analysis and I only got back to it now.

On Aug  1, 2012, Richard Guenther <richard.guenther@gmail.com> wrote:

> On Fri, Dec 2, 2011 at 8:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> 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.

> But we do _not_ want to
> have used non-register (but register type) variables tracked because we
> do not track aliases?

The reasoning, back when I wrote target_for_debug_bind, was that it
didn't make sense to enable VTA for variables that were addressable,
because the location of addressable variables isn't subject to change.
Since their location is a constant address, we're better off avoiding
all the VTA location wrangling and just using their unchanging MEM
location.

> -  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;

This change, although not incorrect, would subject lots of variables
unnecessary VTA treatment, growing memory use and compile time.
diff mbox

Patch

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;
 }