[debug] Add fdebug-nops

Message ID 20180724145910.6lezt2hw6hekuhoh@delia
State New
Headers show
Series
  • [debug] Add fdebug-nops
Related show

Commit Message

Tom de Vries July 24, 2018, 2:59 p.m.
On Tue, Jul 24, 2018 at 01:35:14PM +0200, Tom de Vries wrote:
> On Tue, Jul 24, 2018 at 01:30:30PM +0200, Tom de Vries wrote:

> > On 07/16/2018 05:10 PM, Tom de Vries wrote:

> > > On 07/16/2018 03:50 PM, Richard Biener wrote:

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

> > >>> Any comments?

> > >>

> > >> Interesting idea.  I wonder if that should be generalized

> > >> to other places

> > > 

> > > I kept the option name general, to allow for that.

> > > 

> > > And indeed, this is a point-fix patch. I've been playing around with a

> > > more generic patch that adds nops such that each is_stmt .loc is

> > > associated with a unique insn, but that was embedded in an

> > > fkeep-vars-live branch, so this patch is minimally addressing the first

> > > problem I managed to reproduce on trunk.

> > > 

> > >> and how we can avoid compare-debug failures

> > >> (var-tracking usually doesn't change code-generation).

> > >>

> > > 

> > 

> > I'll post this patch series (the current state of my fkeep-vars-live

> > branch) in reply to this email:

> > 

> >      1  [debug] Add fdebug-nops

> >      2  [debug] Add fkeep-vars-live

> >      3  [debug] Add fdebug-nops and fkeep-vars-live to Og only

> > 

> > Bootstrapped and reg-tested on x86_64. ChangeLog entries and function

> > header comments missing.

> > 

> > Comments welcome.

> > 

> 


And here's a version that is fcompare-debug friendly.

OK for trunk?

Thanks,
- Tom

[debug] Add fdebug-nops

Consider 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 after the ".loc is_stmt 1" annotation:
...
        .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;
...

The option should have the same effect as using a debugger with location views
capability.

2018-07-24  Tom de Vries  <tdevries@suse.de>

	PR debug/78685
	* common.opt (fdebug-nops): New option.
	* passes.def (pass_late_compilation): Add pass_debug_nops.
	* rtl.h (MAY_HAVE_DEBUG_MARKER_INSNS): Add flag_debug_nops.
	* timevar.def (TV_VAR_DEBUG_NOPS): New timevar.
	* tree-pass.h (make_pass_debug_nops): Declare.
	* tree.h (MAY_HAVE_DEBUG_MARKER_STMTS): Add flag_debug_nops.
	* var-tracking.c (insert_debug_nops): New function.
	(class pass_debug_nops): New pass.
	(make_pass_debug_nops): New function.

	* gcc.dg/guality/pr54200-2.c: New test.

---
 gcc/common.opt                           |  4 ++
 gcc/passes.def                           |  1 +
 gcc/rtl.h                                |  2 +-
 gcc/testsuite/gcc.dg/guality/pr54200-2.c | 37 +++++++++++++
 gcc/timevar.def                          |  1 +
 gcc/tree-pass.h                          |  1 +
 gcc/tree.h                               |  2 +-
 gcc/var-tracking.c                       | 91 ++++++++++++++++++++++++++++++++
 8 files changed, 137 insertions(+), 2 deletions(-)

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 4bf8de90331..984b351cd79 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1492,6 +1492,10 @@  Common Report Var(flag_hoist_adjacent_loads) Optimization
 Enable hoisting adjacent loads to encourage generating conditional move
 instructions.
 
+fdebug-nops
+Common Report Var(flag_debug_nops) Optimization
+Insert nops if that might improve 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/passes.def b/gcc/passes.def
index 2a8fbc2efbe..e97c4f9533a 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -487,6 +487,7 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_late_compilation);
       PUSH_INSERT_PASSES_WITHIN (pass_late_compilation)
 	  NEXT_PASS (pass_compute_alignments);
+	  NEXT_PASS (pass_debug_nops);
 	  NEXT_PASS (pass_variable_tracking);
 	  NEXT_PASS (pass_free_cfg);
 	  NEXT_PASS (pass_machine_reorg);
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 565ce3abbe4..29a2e19cb6e 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -843,7 +843,7 @@  struct GTY(()) rtvec_def {
 #define NONDEBUG_INSN_P(X) (INSN_P (X) && !DEBUG_INSN_P (X))
 
 /* Nonzero if DEBUG_MARKER_INSN_P may possibly hold.  */
-#define MAY_HAVE_DEBUG_MARKER_INSNS debug_nonbind_markers_p
+#define MAY_HAVE_DEBUG_MARKER_INSNS (debug_nonbind_markers_p || flag_debug_nops)
 /* Nonzero if DEBUG_BIND_INSN_P may possibly hold.  */
 #define MAY_HAVE_DEBUG_BIND_INSNS flag_var_tracking_assignments
 /* Nonzero if DEBUG_INSN_P may possibly hold.  */
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..e30e3c92b94
--- /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 -fdebug-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/timevar.def b/gcc/timevar.def
index 91221ae5b0e..10216c669de 100644
--- a/gcc/timevar.def
+++ b/gcc/timevar.def
@@ -286,6 +286,7 @@  DEFTIMEVAR (TV_SYMOUT                , "symout")
 DEFTIMEVAR (TV_VAR_TRACKING          , "variable tracking")
 DEFTIMEVAR (TV_VAR_TRACKING_DATAFLOW , "var-tracking dataflow")
 DEFTIMEVAR (TV_VAR_TRACKING_EMIT     , "var-tracking emit")
+DEFTIMEVAR (TV_VAR_DEBUG_NOPS        , "debug nops emit")
 DEFTIMEVAR (TV_TREE_IFCOMBINE        , "tree if-combine")
 DEFTIMEVAR (TV_TREE_UNINIT           , "uninit var analysis")
 DEFTIMEVAR (TV_PLUGIN_INIT           , "plugin initialization")
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index af15adc8e0c..9cce0e1a7cd 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -603,6 +603,7 @@  extern rtl_opt_pass *make_pass_stack_regs_run (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_df_finish (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_compute_alignments (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_duplicate_computed_gotos (gcc::context *ctxt);
+extern rtl_opt_pass *make_pass_debug_nops (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_variable_tracking (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_free_cfg (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_machine_reorg (gcc::context *ctxt);
diff --git a/gcc/tree.h b/gcc/tree.h
index 70ac78130c0..67985617863 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1126,7 +1126,7 @@  extern void omp_clause_range_check_failed (const_tree, const char *, int,
   ((int)TREE_INT_CST_LOW (VL_EXP_CHECK (NODE)->exp.operands[0]))
 
 /* Nonzero if gimple_debug_nonbind_marker_p() may possibly hold.  */
-#define MAY_HAVE_DEBUG_MARKER_STMTS debug_nonbind_markers_p
+#define MAY_HAVE_DEBUG_MARKER_STMTS (debug_nonbind_markers_p || flag_debug_nops)
 /* Nonzero if gimple_debug_bind_p() (and thus
    gimple_debug_source_bind_p()) may possibly hold.  */
 #define MAY_HAVE_DEBUG_BIND_STMTS flag_var_tracking_assignments
diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 5537fa64085..04e878610e9 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10559,3 +10559,94 @@  make_pass_variable_tracking (gcc::context *ctxt)
 {
   return new pass_variable_tracking (ctxt);
 }
+
+/* Insert nop after debug marker insn, if that insn is not followed by a real
+   insn on the same line.  */
+
+static void
+insert_debug_nops (void)
+{
+  rtx_insn *debug_marker = NULL;
+
+  for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
+    {
+      bool use_p = INSN_P (insn) && GET_CODE (PATTERN (insn)) == USE;
+      bool clobber_p = INSN_P (insn) && GET_CODE (PATTERN (insn)) == CLOBBER;
+
+      if (!debug_marker)
+	{
+	  if (DEBUG_MARKER_INSN_P (insn))
+	    debug_marker = insn;
+	  continue;
+	}
+
+      if (LABEL_P (insn) || BARRIER_P (insn) || DEBUG_MARKER_INSN_P (insn))
+	;
+      else if (use_p || clobber_p || NOTE_P (insn) || DEBUG_INSN_P (insn)
+	       || JUMP_TABLE_DATA_P (insn))
+	continue;
+      else if (INSN_P (insn))
+	{
+	  unsigned int debug_marker_loc = INSN_LOCATION (debug_marker);
+	  unsigned int insn_loc = INSN_LOCATION (insn);
+
+	  if (LOCATION_FILE (insn_loc) == LOCATION_FILE (debug_marker_loc)
+	      && LOCATION_LINE (insn_loc) == LOCATION_LINE (debug_marker_loc))
+	    {
+	      debug_marker = NULL;
+	      continue;
+	    }
+	}
+      else
+	gcc_unreachable ();
+
+      emit_insn_after_setloc (gen_nop (), debug_marker,
+			      INSN_LOCATION (debug_marker));
+      debug_marker = DEBUG_MARKER_INSN_P (insn) ? insn : NULL;
+    }
+}
+
+namespace {
+
+const pass_data pass_data_debug_nops =
+{
+  RTL_PASS, /* type */
+  "debugnops", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_VAR_DEBUG_NOPS, /* tv_id */
+  0, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
+};
+
+class pass_debug_nops : public rtl_opt_pass
+{
+public:
+  pass_debug_nops (gcc::context *ctxt)
+    : rtl_opt_pass (pass_data_debug_nops, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  virtual bool gate (function *)
+    {
+      return flag_debug_nops;
+    }
+
+  virtual unsigned int execute (function *)
+    {
+      insert_debug_nops ();
+      /* If !debug_nonbind_markers_p, we can drop those insn here.  */
+      return 0;
+    }
+
+}; // class pass_debug_nops
+
+} // anon namespace
+
+rtl_opt_pass *
+make_pass_debug_nops (gcc::context *ctxt)
+{
+  return new pass_debug_nops (ctxt);
+}