[patch] Fix PR40012, invalid debug locations

[Richard Guenther]

On Thu, Jun 4, 2009 at 4:13 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> since expand from SSA we sometimes expanded DECLs twice, once for the SSA
> partitions, and once when walking over the BLOCK tree.  The former was
> used in code, and the latter in debug information at -O0.  Apart from
> taking up stack space needlessly this screwed debug information.
>
> With optimization this was somewhat remedied by var-tracking (as that one
> looks into the insns and notices the other places), but at -O0 we rely on
> DECL_RTL only.
>
> To fix this I make use of the invariant that all SSA partitions for the
> same basevar are merged together at -O0, hence there's one partition for a
> decl, so we only have one place which we then can as well remember also in
> DECL_RTL for the debug info writer.  This also works at higher
> optimization levels when it so happens that all partitions were
> coalescable.
>
> For the case were we have multiple partitions for one base decl we need to
> detect that it lives in multiple places (I use pc_rtx as marker and reset
> all of this when all variables are layed out).
>
> I checked that the testcases in that bugreport and from Paul at
> http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00190.html give the expected
> output on x86_64-linux, i.e. a DWARF platform.  I would appreciate someone
> checking this also on stabs.  I looked at the asm for stabs and it seemed
> reasonable, but I might be wrong.
>
> I regstrapped this on x86_64-linux with Ada, no regressions.  Okay for
> trunk?

Ok.

Thanks,
Richard.

>
> Ciao,
> Michael.
> --
>        PR debug/40012
>
>        * cfgexpand.c (set_rtl): Store place also in DECL_RTL, if all
>        partitions use the same.
>        (expand_one_var): Deal with DECL_RTL sometimes begin set also
>        for basevars of SSA_NAMEs.
>        (expand_used_vars): Reset TREE_USED for basevars of SSA_NAMEs,
>        to not expand them twice.
>        (gimple_expand_cfg): Clear DECL_RTL for those decls that have
>        multiple places.
>
> Index: cfgexpand.c
> ===================================================================
> --- cfgexpand.c (revision 148164)
> +++ cfgexpand.c (working copy)
> @@ -455,6 +455,28 @@ set_rtl (tree t, rtx x)
>       SA.partition_to_pseudo[var_to_partition (SA.map, t)] = x;
>       if (x && !MEM_P (x))
>        set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (t), x);
> +      /* For the benefit of debug information at -O0 (where vartracking
> +         doesn't run) record the place also in the base DECL if it's
> +        a normal variable (not a parameter).  */
> +      if (x && x != pc_rtx && TREE_CODE (SSA_NAME_VAR (t)) == VAR_DECL)
> +       {
> +         tree var = SSA_NAME_VAR (t);
> +         /* If we don't yet have something recorded, just record it now.  */
> +         if (!DECL_RTL_SET_P (var))
> +           SET_DECL_RTL (var, x);
> +         /* If we have it set alrady to "multiple places" don't
> +            change this.  */
> +         else if (DECL_RTL (var) == pc_rtx)
> +           ;
> +         /* If we have something recorded and it's not the same place
> +            as we want to record now, we have multiple partitions for the
> +            same base variable, with different places.  We can't just
> +            randomly chose one, hence we have to say that we don't know.
> +            This only happens with optimization, and there var-tracking
> +            will figure out the right thing.  */
> +         else if (DECL_RTL (var) != x)
> +           SET_DECL_RTL (var, pc_rtx);
> +       }
>     }
>   else
>     SET_DECL_RTL (t, x);
> @@ -1161,7 +1183,6 @@ expand_one_var (tree var, bool toplevel,
>                  || (!DECL_EXTERNAL (var)
>                      && !DECL_HAS_VALUE_EXPR_P (var)
>                      && !TREE_STATIC (var)
> -                     && !DECL_RTL_SET_P (var)
>                      && TREE_TYPE (var) != error_mark_node
>                      && !DECL_HARD_REGISTER (var)
>                      && really_expand));
> @@ -1174,7 +1195,7 @@ expand_one_var (tree var, bool toplevel,
>     ;
>   else if (TREE_STATIC (var))
>     ;
> -  else if (DECL_RTL_SET_P (var))
> +  else if (TREE_CODE (origvar) != SSA_NAME && DECL_RTL_SET_P (var))
>     ;
>   else if (TREE_TYPE (var) == error_mark_node)
>     {
> @@ -1561,7 +1582,11 @@ expand_used_vars (void)
>
>       /* Expanded above already.  */
>       if (is_gimple_reg (var))
> -       ;
> +       {
> +         TREE_USED (var) = 0;
> +         ggc_free (t);
> +         continue;
> +       }
>       /* We didn't set a block for static or extern because it's hard
>         to tell the difference between a global variable (re)declared
>         in a local scope, and one that's really declared there to
> @@ -2495,6 +2520,12 @@ gimple_expand_cfg (void)
>          && !SA.partition_to_pseudo[i])
>        SA.partition_to_pseudo[i] = DECL_RTL_IF_SET (var);
>       gcc_assert (SA.partition_to_pseudo[i]);
> +
> +      /* If this decl was marked as living in multiple places, reset
> +         this now to NULL.  */
> +      if (DECL_RTL_IF_SET (var) == pc_rtx)
> +       SET_DECL_RTL (var, NULL);
> +
>       /* Some RTL parts really want to look at DECL_RTL(x) when x
>          was a decl marked in REG_ATTR or MEM_ATTR.  We could use
>         SET_DECL_RTL here making this available, but that would mean
>