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 <tdevr...@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(-)
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);
+}
