* [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779)
@ 2016-01-18 23:33 Jakub Jelinek
2016-01-19 12:27 ` Bernd Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2016-01-18 23:33 UTC (permalink / raw)
To: Bernd Schmidt, Jeff Law, Alexandre Oliva; +Cc: gcc-patches
Hi!
On the following testcase with -mrelocatable on ppc32 we get assembly that
contains undefined reference to a local .LC* symbol.
The problem is that prepare_shrink_wrap attempts to schedule some
instructions from the entry block to later basic blocks, if they set a
register that is only used in one of the paths, but the debug infos are kept
where they used to appear (that is correct thing to do), but nothing adjusts
them or resets them); on most targets we get away just with wrong debug
info, but on ppc32 -mrelocatable something in the prologue sets the hard
registers used by some debug insns to subtraction of two magic labels, which
are later removed as unneeded, but kept in the debug info.
Fixed by using the infrastructure we have for this in valtrack.c, that is
used by DCE and DF note problem computation.
E.g. on ppc32, we used to have:
(insn 34 38 35 2 (set (reg/v:SI 6 6 [orig:238 s1 ] [238])
(and:SI (reg/v:SI 3 3 [orig:264 adler ] [264])
(const_int 65535 [0xffff]))) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootloader/../../../powerpc/sha
(nil))
(debug_insn 35 34 36 2 (var_location:SI s1 (reg/v:SI 6 6 [orig:238 s1 ] [238])) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_
(nil))
(insn 36 35 37 2 (set (reg/v:SI 0 0 [orig:239 s2 ] [239])
(lshiftrt:SI (reg/v:SI 3 3 [orig:264 adler ] [264])
(const_int 16 [0x10]))) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootloader/../../../powerpc/shared/b
(nil))
(debug_insn 37 36 39 2 (var_location:SI s2 (reg/v:SI 0 0 [orig:239 s2 ] [239])) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_
(nil))
in bb2 and decide to move insn 34 and 36 to the start of bb3,
before the patch we'd keep the debug insns untouched, while with the
patch we get:
(debug_insn 365 38 35 2 (var_location:SI D#38 (and:SI (reg/v:SI 3 3 [orig:264 adler ] [264])
(const_int 65535 [0xffff]))) -1
(nil))
(debug_insn 35 365 363 2 (var_location:SI s1 (debug_expr:SI D#38)) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootl
(nil))
(debug_insn 363 35 37 2 (var_location:SI D#37 (lshiftrt:SI (reg/v:SI 3 3 [orig:264 adler ] [264])
(const_int 16 [0x10]))) -1
(nil))
(debug_insn 37 363 39 2 (var_location:SI s2 (debug_expr:SI D#37)) ../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootlo
(nil))
in bb2. Even on x86_64-linux I saw even on this testcase improvement
in debug info coverage (it didn't end up being wrong debug there, but
one of the variables used to be <optimized out> without the patch in
certain range.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-01-19 Jakub Jelinek <jakub@redhat.com>
PR debug/65779
* shrink-wrap.c: Include valtrack.h.
(move_insn_for_shrink_wrap): Add DEBUG argument. If
MAY_HAVE_DEBUG_INSNS, call dead_debug_add on DEBUG_INSNs
in between insn and where it will be moved to. Call
dead_debug_insert_temp.
(prepare_shrink_wrap): Adjust caller. Call dead_debug_local_init
first and dead_debug_local_finish at the end.
For uses and defs bitmap, handle all regs in between REGNO and
END_REGNO, not just the first one.
* gcc.dg/pr65779.c: New test.
--- gcc/shrink-wrap.c.jj 2016-01-08 07:31:10.000000000 +0100
+++ gcc/shrink-wrap.c 2016-01-18 20:06:53.029487254 +0100
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.
#include "shrink-wrap.h"
#include "regcprop.h"
#include "rtl-iter.h"
+#include "valtrack.h"
/* Return true if INSN requires the stack frame to be set up.
@@ -149,7 +150,8 @@ static bool
move_insn_for_shrink_wrap (basic_block bb, rtx_insn *insn,
const HARD_REG_SET uses,
const HARD_REG_SET defs,
- bool *split_p)
+ bool *split_p,
+ struct dead_debug_local *debug)
{
rtx set, src, dest;
bitmap live_out, live_in, bb_uses, bb_defs;
@@ -158,6 +160,8 @@ move_insn_for_shrink_wrap (basic_block b
unsigned int end_sregno = FIRST_PSEUDO_REGISTER;
basic_block next_block;
edge live_edge;
+ rtx_insn *dinsn;
+ df_ref def;
/* Look for a simple register assignment. We don't use single_set here
because we can't deal with any CLOBBERs, USEs, or REG_UNUSED secondary
@@ -298,6 +302,19 @@ move_insn_for_shrink_wrap (basic_block b
*split_p = true;
}
+ if (MAY_HAVE_DEBUG_INSNS)
+ {
+ for (dinsn = BB_END (bb); dinsn != insn; dinsn = PREV_INSN (dinsn))
+ if (DEBUG_INSN_P (dinsn))
+ {
+ df_ref use;
+ FOR_EACH_INSN_USE (use, dinsn)
+ if (refers_to_regno_p (dregno, end_dregno,
+ DF_REF_REG (use), (rtx *) NULL))
+ dead_debug_add (debug, use, DF_REF_REGNO (use));
+ }
+ }
+
/* At this point we are committed to moving INSN, but let's try to
move it as far as we can. */
do
@@ -363,6 +380,18 @@ move_insn_for_shrink_wrap (basic_block b
if (!live_edge || EDGE_COUNT (live_edge->dest->preds) > 1)
break;
next_block = live_edge->dest;
+ if (MAY_HAVE_DEBUG_INSNS)
+ {
+ FOR_BB_INSNS_REVERSE (bb, dinsn)
+ if (DEBUG_INSN_P (dinsn))
+ {
+ df_ref use;
+ FOR_EACH_INSN_USE (use, dinsn)
+ if (refers_to_regno_p (dregno, end_dregno,
+ DF_REF_REG (use), (rtx *) NULL))
+ dead_debug_add (debug, use, DF_REF_REGNO (use));
+ }
+ }
}
}
while (next_block);
@@ -384,6 +413,12 @@ move_insn_for_shrink_wrap (basic_block b
SET_REGNO_REG_SET (bb_uses, i);
}
+ /* Insert debug temps for dead REGs used in subsequent debug insns. */
+ if (debug->used && !bitmap_empty_p (debug->used))
+ FOR_EACH_INSN_DEF (def, insn)
+ dead_debug_insert_temp (debug, DF_REF_REGNO (def), insn,
+ DEBUG_TEMP_BEFORE_WITH_VALUE);
+
emit_insn_after (PATTERN (insn), bb_note (bb));
delete_insn (insn);
return true;
@@ -404,6 +439,8 @@ prepare_shrink_wrap (basic_block entry_b
HARD_REG_SET uses, defs;
df_ref def, use;
bool split_p = false;
+ unsigned int i;
+ struct dead_debug_local debug;
if (JUMP_P (BB_END (entry_block)))
{
@@ -414,19 +451,22 @@ prepare_shrink_wrap (basic_block entry_b
copyprop_hardreg_forward_bb_without_debug_insn (entry_block);
}
+ dead_debug_local_init (&debug, NULL, NULL);
CLEAR_HARD_REG_SET (uses);
CLEAR_HARD_REG_SET (defs);
+
FOR_BB_INSNS_REVERSE_SAFE (entry_block, insn, curr)
if (NONDEBUG_INSN_P (insn)
&& !move_insn_for_shrink_wrap (entry_block, insn, uses, defs,
- &split_p))
+ &split_p, &debug))
{
/* Add all defined registers to DEFs. */
FOR_EACH_INSN_DEF (def, insn)
{
x = DF_REF_REG (def);
if (REG_P (x) && HARD_REGISTER_P (x))
- SET_HARD_REG_BIT (defs, REGNO (x));
+ for (i = REGNO (x); i < END_REGNO (x); i++)
+ SET_HARD_REG_BIT (defs, i);
}
/* Add all used registers to USESs. */
@@ -434,9 +474,12 @@ prepare_shrink_wrap (basic_block entry_b
{
x = DF_REF_REG (use);
if (REG_P (x) && HARD_REGISTER_P (x))
- SET_HARD_REG_BIT (uses, REGNO (x));
+ for (i = REGNO (x); i < END_REGNO (x); i++)
+ SET_HARD_REG_BIT (uses, i);
}
}
+
+ dead_debug_local_finish (&debug, NULL);
}
/* Return whether basic block PRO can get the prologue. It can not if it
--- gcc/testsuite/gcc.dg/pr65779.c.jj 2016-01-18 20:27:48.906187675 +0100
+++ gcc/testsuite/gcc.dg/pr65779.c 2016-01-18 20:20:02.000000000 +0100
@@ -0,0 +1,42 @@
+/* PR debug/65779 */
+/* { dg-do assemble } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+unsigned long
+foo (unsigned long x, unsigned char *y, unsigned int z)
+{
+ unsigned long a = x & 0xffff;
+ unsigned long b = (x >> 16) & 0xffff;
+ int k;
+ if (y == 0) return 1L;
+ while (z > 0)
+ {
+ k = z < 5552 ? z : 5552;
+ z -= k;
+ while (k >= 16)
+ {
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ k -= 16;
+ }
+ if (k != 0)
+ do { a += *y++; b += a; } while (--k);
+ a %= 65521L;
+ b %= 65521L;
+ }
+ return (b << 16) | a;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779)
2016-01-18 23:33 [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779) Jakub Jelinek
@ 2016-01-19 12:27 ` Bernd Schmidt
2016-01-19 13:08 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Bernd Schmidt @ 2016-01-19 12:27 UTC (permalink / raw)
To: Jakub Jelinek, Jeff Law, Alexandre Oliva; +Cc: gcc-patches
On 01/19/2016 12:33 AM, Jakub Jelinek wrote:
> + if (MAY_HAVE_DEBUG_INSNS)
> + {
> + for (dinsn = BB_END (bb); dinsn != insn; dinsn = PREV_INSN (dinsn))
> + if (DEBUG_INSN_P (dinsn))
> + {
> + df_ref use;
> + FOR_EACH_INSN_USE (use, dinsn)
> + if (refers_to_regno_p (dregno, end_dregno,
> + DF_REF_REG (use), (rtx *) NULL))
> + dead_debug_add (debug, use, DF_REF_REGNO (use));
> + }
> + }
> +
> /* At this point we are committed to moving INSN, but let's try to
> move it as far as we can. */
> do
> @@ -363,6 +380,18 @@ move_insn_for_shrink_wrap (basic_block b
> if (!live_edge || EDGE_COUNT (live_edge->dest->preds) > 1)
> break;
> next_block = live_edge->dest;
> + if (MAY_HAVE_DEBUG_INSNS)
> + {
> + FOR_BB_INSNS_REVERSE (bb, dinsn)
> + if (DEBUG_INSN_P (dinsn))
> + {
> + df_ref use;
> + FOR_EACH_INSN_USE (use, dinsn)
> + if (refers_to_regno_p (dregno, end_dregno,
> + DF_REF_REG (use), (rtx *) NULL))
> + dead_debug_add (debug, use, DF_REF_REGNO (use));
> + }
> + }
> }
> }
Is there a way to merge these two blocks (e.g. by moving this to the
start of the loop and testing for insn or BB_HEAD)?
Bernd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779)
2016-01-19 12:27 ` Bernd Schmidt
@ 2016-01-19 13:08 ` Jakub Jelinek
2016-01-19 13:12 ` Bernd Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2016-01-19 13:08 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: Jeff Law, Alexandre Oliva, gcc-patches
On Tue, Jan 19, 2016 at 01:27:32PM +0100, Bernd Schmidt wrote:
> Is there a way to merge these two blocks (e.g. by moving this to the start
> of the loop and testing for insn or BB_HEAD)?
Sure, like this?
2016-01-19 Jakub Jelinek <jakub@redhat.com>
PR debug/65779
* shrink-wrap.c: Include valtrack.h.
(move_insn_for_shrink_wrap): Add DEBUG argument. If
MAY_HAVE_DEBUG_INSNS, call dead_debug_add on DEBUG_INSNs
in between insn and where it will be moved to. Call
dead_debug_insert_temp.
(prepare_shrink_wrap): Adjust caller. Call dead_debug_local_init
first and dead_debug_local_finish at the end.
For uses and defs bitmap, handle all regs in between REGNO and
END_REGNO, not just the first one.
* gcc.dg/pr65779.c: New test.
--- gcc/shrink-wrap.c.jj 2016-01-19 09:20:22.525791441 +0100
+++ gcc/shrink-wrap.c 2016-01-19 13:56:18.256816084 +0100
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.
#include "shrink-wrap.h"
#include "regcprop.h"
#include "rtl-iter.h"
+#include "valtrack.h"
/* Return true if INSN requires the stack frame to be set up.
@@ -149,7 +150,8 @@ static bool
move_insn_for_shrink_wrap (basic_block bb, rtx_insn *insn,
const HARD_REG_SET uses,
const HARD_REG_SET defs,
- bool *split_p)
+ bool *split_p,
+ struct dead_debug_local *debug)
{
rtx set, src, dest;
bitmap live_out, live_in, bb_uses, bb_defs;
@@ -158,6 +160,8 @@ move_insn_for_shrink_wrap (basic_block b
unsigned int end_sregno = FIRST_PSEUDO_REGISTER;
basic_block next_block;
edge live_edge;
+ rtx_insn *dinsn;
+ df_ref def;
/* Look for a simple register assignment. We don't use single_set here
because we can't deal with any CLOBBERs, USEs, or REG_UNUSED secondary
@@ -302,6 +306,20 @@ move_insn_for_shrink_wrap (basic_block b
move it as far as we can. */
do
{
+ if (MAY_HAVE_DEBUG_INSNS)
+ {
+ FOR_BB_INSNS_REVERSE (bb, dinsn)
+ if (DEBUG_INSN_P (dinsn))
+ {
+ df_ref use;
+ FOR_EACH_INSN_USE (use, dinsn)
+ if (refers_to_regno_p (dregno, end_dregno,
+ DF_REF_REG (use), (rtx *) NULL))
+ dead_debug_add (debug, use, DF_REF_REGNO (use));
+ }
+ else if (dinsn == insn)
+ break;
+ }
live_out = df_get_live_out (bb);
live_in = df_get_live_in (next_block);
bb = next_block;
@@ -384,6 +402,12 @@ move_insn_for_shrink_wrap (basic_block b
SET_REGNO_REG_SET (bb_uses, i);
}
+ /* Insert debug temps for dead REGs used in subsequent debug insns. */
+ if (debug->used && !bitmap_empty_p (debug->used))
+ FOR_EACH_INSN_DEF (def, insn)
+ dead_debug_insert_temp (debug, DF_REF_REGNO (def), insn,
+ DEBUG_TEMP_BEFORE_WITH_VALUE);
+
emit_insn_after (PATTERN (insn), bb_note (bb));
delete_insn (insn);
return true;
@@ -404,6 +428,8 @@ prepare_shrink_wrap (basic_block entry_b
HARD_REG_SET uses, defs;
df_ref def, use;
bool split_p = false;
+ unsigned int i;
+ struct dead_debug_local debug;
if (JUMP_P (BB_END (entry_block)))
{
@@ -414,19 +440,22 @@ prepare_shrink_wrap (basic_block entry_b
copyprop_hardreg_forward_bb_without_debug_insn (entry_block);
}
+ dead_debug_local_init (&debug, NULL, NULL);
CLEAR_HARD_REG_SET (uses);
CLEAR_HARD_REG_SET (defs);
+
FOR_BB_INSNS_REVERSE_SAFE (entry_block, insn, curr)
if (NONDEBUG_INSN_P (insn)
&& !move_insn_for_shrink_wrap (entry_block, insn, uses, defs,
- &split_p))
+ &split_p, &debug))
{
/* Add all defined registers to DEFs. */
FOR_EACH_INSN_DEF (def, insn)
{
x = DF_REF_REG (def);
if (REG_P (x) && HARD_REGISTER_P (x))
- SET_HARD_REG_BIT (defs, REGNO (x));
+ for (i = REGNO (x); i < END_REGNO (x); i++)
+ SET_HARD_REG_BIT (defs, i);
}
/* Add all used registers to USESs. */
@@ -434,9 +463,12 @@ prepare_shrink_wrap (basic_block entry_b
{
x = DF_REF_REG (use);
if (REG_P (x) && HARD_REGISTER_P (x))
- SET_HARD_REG_BIT (uses, REGNO (x));
+ for (i = REGNO (x); i < END_REGNO (x); i++)
+ SET_HARD_REG_BIT (uses, i);
}
}
+
+ dead_debug_local_finish (&debug, NULL);
}
/* Return whether basic block PRO can get the prologue. It can not if it
--- gcc/testsuite/gcc.dg/pr65779.c.jj 2016-01-19 13:53:13.534358036 +0100
+++ gcc/testsuite/gcc.dg/pr65779.c 2016-01-19 13:53:13.534358036 +0100
@@ -0,0 +1,42 @@
+/* PR debug/65779 */
+/* { dg-do assemble } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+unsigned long
+foo (unsigned long x, unsigned char *y, unsigned int z)
+{
+ unsigned long a = x & 0xffff;
+ unsigned long b = (x >> 16) & 0xffff;
+ int k;
+ if (y == 0) return 1L;
+ while (z > 0)
+ {
+ k = z < 5552 ? z : 5552;
+ z -= k;
+ while (k >= 16)
+ {
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ a += *y++; b += a;
+ k -= 16;
+ }
+ if (k != 0)
+ do { a += *y++; b += a; } while (--k);
+ a %= 65521L;
+ b %= 65521L;
+ }
+ return (b << 16) | a;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779)
2016-01-19 13:08 ` Jakub Jelinek
@ 2016-01-19 13:12 ` Bernd Schmidt
0 siblings, 0 replies; 4+ messages in thread
From: Bernd Schmidt @ 2016-01-19 13:12 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, Alexandre Oliva, gcc-patches
On 01/19/2016 02:08 PM, Jakub Jelinek wrote:
> On Tue, Jan 19, 2016 at 01:27:32PM +0100, Bernd Schmidt wrote:
>> Is there a way to merge these two blocks (e.g. by moving this to the start
>> of the loop and testing for insn or BB_HEAD)?
>
> Sure, like this?
That's ok. I'm assuming you know best how to use the dead_debug stuff.
Bernd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-19 13:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-18 23:33 [PATCH] Fix debug info handling in prepare_shrink_wrap (PR debug/65779) Jakub Jelinek
2016-01-19 12:27 ` Bernd Schmidt
2016-01-19 13:08 ` Jakub Jelinek
2016-01-19 13:12 ` Bernd Schmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).