Improve expansion into DEBUG_IMPLICIT_PTR (PR debug/54970)

Hi!

This patch improves debug info quality in two small changes.
One is that for DEBUG stmts like DEBUG p => &MEM[&a, 8] for
a non-addressable we were giving up instead of creating DEBUG_IMPLICIT_PTR
(plus 8) for it.  The reason is
          if (GET_CODE (op0) == DEBUG_IMPLICIT_PTR
              || (GET_CODE (op0) == PLUS
                  && GET_CODE (XEXP (op0, 0)) == DEBUG_IMPLICIT_PTR))
            /* (mem (debug_implicit_ptr)) might confuse aliasing.
               Instead just use get_inner_reference.  */
            goto component_ref;
hunk in expand_debug_expr MEM_REF handling which was added to avoid
crashes on some testcases.  The above means unfortunately that while
&a expands into DEBUG_IMPLICIT_PTR, we later throw that away, for MEM_REF
return NULL and for the outer ADDR_EXPR, as what it points to is not
a handled component of non-addressable decl or non-addressable decl itself,
we give up.  Fixed by special casing ADDR_EXPR of such MEM_REF.

The second change is to fix failures of the new testcase with -m32 -Os -
in that case tree-sra.c is considering adding DECL_DEBUG_EXPR, but doesn't,
because MEM_REF isn't handled component.  Fixed by handling MEM_REF with
ADDR_EXPR in the first operand the same way as handled_component_p,
get_ref_base_and_extent handles that just fine.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-10-18  Jakub Jelinek  <jakub@redhat.com>

	PR debug/54970
	* cfgexpand.c (expand_debug_expr): Expand &MEM_REF[&var, n]
	as DEBUG_IMPLICIT_PTR + n if &var expands to DEBUG_IMPLICIT_PTR.
	* tree-sra.c (create_access_replacement): Allow also MEM_REFs
	with ADDR_EXPR first operand in DECL_DEBUG_EXPR expressions.
	* var-tracking.c (track_expr_p): Handle MEM_REFs in DECL_DEBUG_EXPR
	expressions.
	* dwarf2out.c (add_var_loc_to_decl): Likewise.

	* gcc.dg/guality/pr54970.c: New test.


	Jakub

On Thu, Oct 18, 2012 at 2:17 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This patch improves debug info quality in two small changes.
> One is that for DEBUG stmts like DEBUG p => &MEM[&a, 8] for
> a non-addressable we were giving up instead of creating DEBUG_IMPLICIT_PTR
> (plus 8) for it.  The reason is
>           if (GET_CODE (op0) == DEBUG_IMPLICIT_PTR
>               || (GET_CODE (op0) == PLUS
>                   && GET_CODE (XEXP (op0, 0)) == DEBUG_IMPLICIT_PTR))
>             /* (mem (debug_implicit_ptr)) might confuse aliasing.
>                Instead just use get_inner_reference.  */
>             goto component_ref;
> hunk in expand_debug_expr MEM_REF handling which was added to avoid
> crashes on some testcases.  The above means unfortunately that while
> &a expands into DEBUG_IMPLICIT_PTR, we later throw that away, for MEM_REF
> return NULL and for the outer ADDR_EXPR, as what it points to is not
> a handled component of non-addressable decl or non-addressable decl itself,
> we give up.  Fixed by special casing ADDR_EXPR of such MEM_REF.
>
> The second change is to fix failures of the new testcase with -m32 -Os -
> in that case tree-sra.c is considering adding DECL_DEBUG_EXPR, but doesn't,
> because MEM_REF isn't handled component.  Fixed by handling MEM_REF with
> ADDR_EXPR in the first operand the same way as handled_component_p,
> get_ref_base_and_extent handles that just fine.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok for the tree-sra.c parts, I'll appreciate another eye on the rest.

Thanks,
Richard.

> 2012-10-18  Jakub Jelinek  <jakub@redhat.com>
>
>         PR debug/54970
>         * cfgexpand.c (expand_debug_expr): Expand &MEM_REF[&var, n]
>         as DEBUG_IMPLICIT_PTR + n if &var expands to DEBUG_IMPLICIT_PTR.
>         * tree-sra.c (create_access_replacement): Allow also MEM_REFs
>         with ADDR_EXPR first operand in DECL_DEBUG_EXPR expressions.
>         * var-tracking.c (track_expr_p): Handle MEM_REFs in DECL_DEBUG_EXPR
>         expressions.
>         * dwarf2out.c (add_var_loc_to_decl): Likewise.
>
>         * gcc.dg/guality/pr54970.c: New test.
>
> --- gcc/cfgexpand.c.jj  2012-10-17 17:18:21.000000000 +0200
> +++ gcc/cfgexpand.c     2012-10-18 10:06:35.058258719 +0200
> @@ -3284,6 +3284,27 @@ expand_debug_expr (tree exp)
>                 }
>             }
>
> +         if (TREE_CODE (TREE_OPERAND (exp, 0)) == MEM_REF
> +             && TREE_CODE (TREE_OPERAND (TREE_OPERAND (exp, 0), 0))
> +                == ADDR_EXPR)
> +           {
> +             op0 = expand_debug_expr (TREE_OPERAND (TREE_OPERAND (exp, 0),
> +                                                    0));
> +             if (op0 != NULL
> +                 && (GET_CODE (op0) == DEBUG_IMPLICIT_PTR
> +                     || (GET_CODE (op0) == PLUS
> +                         && GET_CODE (XEXP (op0, 0)) == DEBUG_IMPLICIT_PTR
> +                         && CONST_INT_P (XEXP (op0, 1)))))
> +               {
> +                 op1 = expand_debug_expr (TREE_OPERAND (TREE_OPERAND (exp, 0),
> +                                                        1));
> +                 if (!op1 || !CONST_INT_P (op1))
> +                   return NULL;
> +
> +                 return plus_constant (mode, op0, INTVAL (op1));
> +               }
> +           }
> +
>           return NULL;
>         }
>
> --- gcc/tree-sra.c.jj   2012-09-21 15:04:35.000000000 +0200
> +++ gcc/tree-sra.c      2012-10-18 11:09:26.310581459 +0200
> @@ -1894,7 +1894,8 @@ create_access_replacement (struct access
>          and that get_ref_base_and_extent works properly on the
>          expression.  It cannot handle accesses at a non-constant offset
>          though, so just give up in those cases.  */
> -      for (d = debug_expr; !fail && handled_component_p (d);
> +      for (d = debug_expr;
> +          !fail && (handled_component_p (d) || TREE_CODE (d) == MEM_REF);
>            d = TREE_OPERAND (d, 0))
>         switch (TREE_CODE (d))
>           {
> @@ -1912,6 +1913,12 @@ create_access_replacement (struct access
>                 && TREE_CODE (TREE_OPERAND (d, 2)) != INTEGER_CST)
>               fail = true;
>             break;
> +         case MEM_REF:
> +           if (TREE_CODE (TREE_OPERAND (d, 0)) != ADDR_EXPR)
> +             fail = true;
> +           else
> +             d = TREE_OPERAND (d, 0);
> +           break;
>           default:
>             break;
>           }
> --- gcc/var-tracking.c.jj       2012-10-17 17:18:20.000000000 +0200
> +++ gcc/var-tracking.c  2012-10-18 11:13:21.300327939 +0200
> @@ -4921,7 +4921,9 @@ track_expr_p (tree expr, bool need_rtl)
>         realdecl = expr;
>        else if (!DECL_P (realdecl))
>         {
> -         if (handled_component_p (realdecl))
> +         if (handled_component_p (realdecl)
> +             || (TREE_CODE (realdecl) == MEM_REF
> +                 && TREE_CODE (TREE_OPERAND (realdecl, 0)) == ADDR_EXPR))
>             {
>               HOST_WIDE_INT bitsize, bitpos, maxsize;
>               tree innerdecl
> --- gcc/dwarf2out.c.jj  2012-10-17 17:18:21.000000000 +0200
> +++ gcc/dwarf2out.c     2012-10-18 11:16:45.504211284 +0200
> @@ -4620,7 +4620,10 @@ add_var_loc_to_decl (tree decl, rtx loc_
>    if (DECL_DEBUG_EXPR_IS_FROM (decl))
>      {
>        tree realdecl = DECL_DEBUG_EXPR (decl);
> -      if (realdecl && handled_component_p (realdecl))
> +      if (realdecl
> +         && (handled_component_p (realdecl)
> +             || (TREE_CODE (realdecl) == MEM_REF
> +                 && TREE_CODE (TREE_OPERAND (realdecl, 0)) == ADDR_EXPR)))
>         {
>           HOST_WIDE_INT maxsize;
>           tree innerdecl;
> --- gcc/testsuite/gcc.dg/guality/pr54970.c.jj   2012-10-18 10:21:25.587109451 +0200
> +++ gcc/testsuite/gcc.dg/guality/pr54970.c      2012-10-18 10:46:40.746395100 +0200
> @@ -0,0 +1,23 @@
> +/* PR debug/54970 */
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +#include "../nop.h"
> +
> +int
> +main ()
> +{
> +  int a[] = { 1, 2, 3 };       /* { dg-final { gdb-test 13 "a\[1\]" "2" } } */
> +  int *p = a + 2;              /* { dg-final { gdb-test 13 "a\[2\]" "3" } } */
> +  int *q = a + 1;              /* { dg-final { gdb-test 13 "*p" "3" } } */
> +  asm volatile ("NOP");                /* { dg-final { gdb-test 13 "*q" "2" } } */
> +  *p += 10;                    /* { dg-final { gdb-test 17 "a\[1\]" "2" } } */
> +                               /* { dg-final { gdb-test 17 "a\[2\]" "13" } } */
> +                               /* { dg-final { gdb-test 17 "*p" "13" } } */
> +  asm volatile ("NOP");                /* { dg-final { gdb-test 17 "*q" "2" } } */
> +  *q += 10;                    /* { dg-final { gdb-test 21 "a\[1\]" "12" } } */
> +                               /* { dg-final { gdb-test 21 "a\[2\]" "13" } } */
> +                               /* { dg-final { gdb-test 21 "*p" "13" } } */
> +  asm volatile ("NOP");                /* { dg-final { gdb-test 21 "*q" "12" } } */
> +  return 0;
> +}
>
>         Jakub

> 2012-10-18  Jakub Jelinek  <jakub@redhat.com>
>
>         PR debug/54970
>         * cfgexpand.c (expand_debug_expr): Expand &MEM_REF[&var, n]
>         as DEBUG_IMPLICIT_PTR + n if &var expands to DEBUG_IMPLICIT_PTR.
>         * tree-sra.c (create_access_replacement): Allow also MEM_REFs
>         with ADDR_EXPR first operand in DECL_DEBUG_EXPR expressions.
>         * var-tracking.c (track_expr_p): Handle MEM_REFs in DECL_DEBUG_EXPR
>         expressions.
>         * dwarf2out.c (add_var_loc_to_decl): Likewise.

OK on the dwarf2out.c part.

-cary

On Oct 18, 2012, Jakub Jelinek <jakub@redhat.com> wrote:

> 2012-10-18  Jakub Jelinek  <jakub@redhat.com>

> 	PR debug/54970
> 	* cfgexpand.c (expand_debug_expr): Expand &MEM_REF[&var, n]
> 	as DEBUG_IMPLICIT_PTR + n if &var expands to DEBUG_IMPLICIT_PTR.
> 	* tree-sra.c (create_access_replacement): Allow also MEM_REFs
> 	with ADDR_EXPR first operand in DECL_DEBUG_EXPR expressions.
> 	* var-tracking.c (track_expr_p): Handle MEM_REFs in DECL_DEBUG_EXPR
> 	expressions.
> 	* dwarf2out.c (add_var_loc_to_decl): Likewise.

var-tracking.c and expand_debug_expr changes are ok.  I guess with this
you got the whole patch approved.