This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[RFC 1/3, debug] Add fdebug-nops
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.
>
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.
There's a design principle in GCC that code generation and debug generation
are independent. This guarantees that if you're encountering a problem in an
application without debug info, you can recompile it with -g and be certain
that you can reproduce the same problem, and use the debug info to debug the
problem. This invariant is enforced by bootstrap-debug. The fdebug-nops
breaks this invariant, but we consider this acceptable since it is intended
for use in combination with -Og -g in the standard edit-compile-debug cycle,
where -g is assumed to be already present at the point of encountering a
problem.
PR debug/78685
[debug] Add fdebug-nops
---
gcc/common.opt | 4 +++
gcc/testsuite/gcc.dg/guality/pr54200-2.c | 37 +++++++++++++++++++++++++
gcc/var-tracking.c | 46 ++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
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/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/var-tracking.c b/gcc/var-tracking.c
index 5537fa64085..1349454d5f2 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10427,6 +10427,49 @@ vt_finalize (void)
vui_allocated = 0;
}
+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;
+ }
+}
+
/* The entry point to variable tracking pass. */
static inline unsigned int
@@ -10434,6 +10477,9 @@ variable_tracking_main_1 (void)
{
bool success;
+ if (flag_debug_nops)
+ insert_debug_nops ();
+
/* We won't be called as a separate pass if flag_var_tracking is not
set, but final may call us to turn debug markers into notes. */
if ((!flag_var_tracking && MAY_HAVE_DEBUG_INSNS)