Re: [RFC][debug] Add -fadd-debug-nops

[Richard Biener <rguenther at suse dot de>]

On Mon, 16 Jul 2018, Tom de Vries wrote:

> Hi,
> 
> this is an idea that I'm currently playing around with: adding nops in
> an optimized application with debug info can improve the debug info.
> 
> 
> Consider f.i. this gdb session in foo of pr54200-2.c (using -Os):
> ...
> (gdb) n
> 26            return a; /* { dg-final { gdb-test . "(int)a" "6" } } */
> (gdb) p a
> 'a' has unknown type; cast it to its declared type
> (gdb) n
> main () at pr54200-2.c:34
> 34        return 0;
> ...
> 
> The problem is that the scope in which a is declared ends at .LBE7, and the
> statement .loc for line 26 ends up attached to the ret insn:
> ...
>         .loc 1 24 11
>         addl    %edx, %eax
> .LVL1:
>         # DEBUG a => ax
>         .loc 1 26 7 is_stmt 1
> .LBE7:
>         .loc 1 28 1 is_stmt 0
>         ret
>         .cfi_endproc
> ...
> 
> This patch fixes the problem (for Og and Os, the 'DEBUG a => ax' is missing
> for O1 and higher) by adding a nop before the ret insn:
> ...
>         .loc 1 24 11
>         addl    %edx, %eax
>  .LVL1:
>         # DEBUG a => ax
>         .loc 1 26 7 is_stmt 1
> +       nop
>  .LBE7:
>         .loc 1 28 1 is_stmt 0
>         ret
>         .cfi_endproc
> ...
> 
> and instead we have:
> ...
> (gdb) n
> 26            return a; /* { dg-final { gdb-test . "(int)a" "6" } } */
> (gdb) p a
> $1 = 6
> (gdb) n
> main () at pr54200-2.c:34
> 34        return 0;
> ...
> 
> Any comments?

Interesting idea.  I wonder if that should be generalized
to other places and how we can avoid compare-debug failures
(var-tracking usually doesn't change code-generation).

Richard.

> Thanks,
> - Tom
> 
> [debug] Add -fadd-debug-nops
> 
> ---
>  gcc/common.opt                           |  4 ++++
>  gcc/testsuite/gcc.dg/guality/pr54200-2.c | 37 ++++++++++++++++++++++++++++++++
>  gcc/var-tracking.c                       | 16 ++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index c29abdb5cb1..8e2876d2b8e 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1504,6 +1504,10 @@ Common Report Var(flag_hoist_adjacent_loads) Optimization
>  Enable hoisting adjacent loads to encourage generating conditional move
>  instructions.
>  
> +fadd-debug-nops
> +Common Report Var(flag_add_debug_nops) Optimization
> +Add nops if that improves debugging of optimized code
> +
>  fkeep-gc-roots-live
>  Common Undocumented Report Var(flag_keep_gc_roots_live) Optimization
>  ; Always keep a pointer to a live memory block
> diff --git a/gcc/testsuite/gcc.dg/guality/pr54200-2.c b/gcc/testsuite/gcc.dg/guality/pr54200-2.c
> new file mode 100644
> index 00000000000..2694f7401e2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/guality/pr54200-2.c
> @@ -0,0 +1,37 @@
> +/* { dg-do run } */
> +/* { dg-skip-if "" { *-*-* }  { "*" } { "-Og" "-Os" "-O0" } } */
> +/* { dg-options "-g -fadd-debug-nops -DMAIN" } */
> +
> +#include "prevent-optimization.h"
> +
> +int o ATTRIBUTE_USED;
> +
> +void
> +bar (void)
> +{
> +  o = 2;
> +}
> +
> +int __attribute__((noinline,noclone))
> +foo (int z, int x, int b)
> +{
> +  if (x == 1)
> +    {
> +      bar ();
> +      return z;
> +    }
> +  else
> +    {
> +      int a = (x + z) + b;
> +      /* Add cast to prevent unsupported.  */
> +      return a; /* { dg-final { gdb-test . "(int)a" "6" } } */
> +    }
> +}
> +
> +#ifdef MAIN
> +int main ()
> +{
> +  foo (3, 2, 1);
> +  return 0;
> +}
> +#endif
> diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
> index 5537fa64085..230845a38a0 100644
> --- a/gcc/var-tracking.c
> +++ b/gcc/var-tracking.c
> @@ -9994,6 +9994,15 @@ reemit_marker_as_note (rtx_insn *insn)
>      }
>  }
>  
> +static void
> +emit_nop_before (rtx_insn *insn)
> +{
> +  rtx_insn *insert_after = PREV_INSN (insn);
> +  while (INSN_LOCATION (insert_after) == INSN_LOCATION (insn))
> +    insert_after = PREV_INSN (insert_after);
> +  emit_insn_after (gen_nop (), insert_after);
> +}
> +
>  /* Allocate and initialize the data structures for variable tracking
>     and parse the RTL to get the micro operations.  */
>  
> @@ -10224,6 +10233,13 @@ vt_initialize (void)
>  		      continue;
>  		    }
>  
> +		  /* Add debug nops before ret insn.  */
> +		  if (optimize
> +		      && flag_add_debug_nops
> +		      && cfun->debug_nonbind_markers
> +		      && returnjump_p (insn))
> +		    emit_nop_before (insn);
> +
>  		  if (MAY_HAVE_DEBUG_BIND_INSNS)
>  		    {
>  		      if (CALL_P (insn))
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)