[PR52983] eliminate autoinc from debug_insn locs (was: Re: fix left-over debug insns in DCE)
My recent patch for PR48866, that introduced dead_debug_insert_temp() with DEBUG_TEMP_BEFORE_WITH_VALUE as a possibility for keeping expressions about to be DCE'd, caused regressions on ppc because it would take MEMs with autoinc addressing modes, which would be rejected down the road. This patch arranges for locs to have autoinc addressing modes eliminated while copying them for use in the debug insns, fixing the problem. On Apr 13, 2012, Paolo Bonzini wrote: > I wonder if it makes any sense to move the dead_debug_* stuff to its own > file... Yeah, that sounds like a reasonable idea, so this patch moves all this value-tracking stuff to valtrack.[ch]. Regstrapped on x86_64-linux-gnu and i686-linux-gnu; verified to fix the reported ppc problem with a cross compiler. Ok to install? for gcc/ChangeLog from Alexandre Oliva PR debug/52983 * valtrack.h, valtrack.c: New. * Makefile.in (VALTRACK_H): New. (OBJS): Add valtrack.o. (valtrack.o): New. (cselib.o, dce.o, df-problems.o, combine.o): Add VALTRACK_H. * combine.c: Include valtrack.h. (make_compound_operation): Publish. (cleanup_auto_inc_dec): Move to valtrack.c. Implement unconditionally, falling back to copy_rtx on non-autoinc machines. (struct rtx_subst_pair, propagate_for_debug_subst): Move to valtrack.c. (propagate_for_debug): Likewise. Add this_basic_block parameter. Adjust all callers. * cselib.c: Include valtrack.h. * dce.c: Likewise. * df-problems.c: Likewise. (dead_debug_init, dead_debug_reset_uses): Move to valtrack.c. (dead_debug_finish, dead_debug_add): Likewise. (dead_debug_insert_temp): Likewise. Use cleanup_auto_inc_dec. * df.h (struct dead_debug_use): Move to valtrack.h. (struct dead_debug, enum debug_temp_where): Likewise. (dead_debug_init, dead_debug_reset_uses): Move to valtrack.h. (dead_debug_finish, dead_debug_add): Likewise. (dead_debug_insert_temp): Likewise. * rtl.h (make_compound_operation): Declare. Index: gcc/Makefile.in === --- gcc/Makefile.in.orig 2012-04-30 08:16:19.496079627 -0300 +++ gcc/Makefile.in 2012-04-30 08:41:04.512982838 -0300 @@ -894,6 +894,7 @@ CGRAPH_H = cgraph.h $(VEC_H) $(TREE_H) $ cif-code.def ipa-ref.h ipa-ref-inline.h $(LINKER_PLUGIN_API_H) DF_H = df.h $(BITMAP_H) $(REGSET_H) sbitmap.h $(BASIC_BLOCK_H) \ alloc-pool.h $(TIMEVAR_H) +VALTRACK_H = valtrack.h $(BITMAP_H) $(DF_H) $(RTL_H) $(BASIC_BLOCK_H) RESOURCE_H = resource.h hard-reg-set.h $(DF_H) DDG_H = ddg.h sbitmap.h $(DF_H) GCC_H = gcc.h version.h $(DIAGNOSTIC_CORE_H) @@ -1437,6 +1438,7 @@ OBJS = \ tree-vectorizer.o \ tree-vrp.o \ tree.o \ + valtrack.o \ value-prof.o \ var-tracking.o \ varasm.o \ @@ -3002,8 +3004,8 @@ coverage.o : coverage.c $(GCOV_IO_H) $(C cselib.o : cselib.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(REGS_H) hard-reg-set.h $(FLAGS_H) insn-config.h $(RECOG_H) \ $(EMIT_RTL_H) $(DIAGNOSTIC_CORE_H) output.h $(FUNCTION_H) $(TREE_PASS_H) \ - cselib.h gt-cselib.h $(GGC_H) $(TM_P_H) $(PARAMS_H) alloc-pool.h \ - $(HASHTAB_H) $(TARGET_H) $(BITMAP_H) + cselib.h gt-cselib.h $(GGC_H) $(TM_P_H) $(VALTRACK_H) $(PARAMS_H) \ + alloc-pool.h $(HASHTAB_H) $(TARGET_H) $(BITMAP_H) cse.o : cse.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) $(REGS_H) \ hard-reg-set.h $(FLAGS_H) insn-config.h $(RECOG_H) $(EXPR_H) toplev.h $(DIAGNOSTIC_CORE_H) \ output.h $(FUNCTION_H) $(BASIC_BLOCK_H) $(GGC_H) $(TM_P_H) $(TIMEVAR_H) \ @@ -3011,8 +3013,8 @@ cse.o : cse.c $(CONFIG_H) $(SYSTEM_H) co $(DF_H) $(DBGCNT_H) dce.o : dce.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(TREE_H) $(REGS_H) hard-reg-set.h $(FLAGS_H) $(EXCEPT_H) $(DF_H) cselib.h \ - $(DBGCNT_H) dce.h $(TIMEVAR_H) $(TREE_PASS_H) $(DBGCNT_H) $(TM_P_H) \ - $(EMIT_RTL_H) + $(DBGCNT_H) dce.h $(VALTRACK_H) $(TIMEVAR_H) $(TREE_PASS_H) \ + $(DBGCNT_H) $(TM_P_H) $(EMIT_RTL_H) dse.o : dse.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(TREE_H) $(TM_P_H) $(REGS_H) hard-reg-set.h $(FLAGS_H) insn-config.h \ $(RECOG_H) $(EXPR_H) $(DF_H) cselib.h $(DBGCNT_H) $(TIMEVAR_H) \ @@ -3104,7 +3106,8 @@ df-core.o : df-core.c $(CONFIG_H) $(SYST df-problems.o : df-problems.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ $(RTL_H) insn-config.h $(RECOG_H) $(FUNCTION_H) $(REGS_H) alloc-pool.h \ hard-reg-set.h $(BASIC_BLOCK_H) $(DF_H) $(BITMAP_H) sbitmap.h $(TIMEVAR_H) \ - $(TM_P_H) $(TARGET_H) $(FLAGS_H) output.h $(EXCEPT_H) dce.h vecprim.h + $(TM_P_H) $(TARGET_H) $(FLAGS_H) output.h $(EXCEPT_H) dce.h \ + vecprim.h $(VALTRACK_H) df-scan.o : df-scan.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ insn-config.h $(RECOG_H) $(FUNCTION_H) $(REGS_H) alloc-pool.h \ hard-reg-set.h $(BASIC_BLOCK_H) $(DF_H) $(BITMAP_H) sbitmap.h $(TIMEVAR_H) \ @@ -3113,6 +3116,8 @@ df-scan.o : df-scan.c $(CONFIG_H) $(SYST regstat.o : regstat.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(TM_P_H) $(FLAG
Re: fix left-over debug insns in DCE
Il 13/04/2012 17:58, Alexandre Oliva ha scritto: > > I've just installed the patch, but if you find the need for any further > improvement, let me know and I'll do it right away. I wonder if it makes any sense to move the dead_debug_* stuff to its own file... Paolo
Re: fix left-over debug insns in DCE
On Apr 9, 2012, Eric Botcazou wrote:
> Could you add a comment for each value?
Done
> Missing "extern" for all declarations.
Thanks, added.
> I don't understand the "or _WITH_VALUE otherwise" part of the comment.
Sorry, my bad. It didn't make sense. Fixed.
> Please add a comment explaining what this is doing.
How's this?
I've just installed the patch, but if you find the need for any further
improvement, let me know and I'll do it right away.
Thanks,
for gcc/ChangeLog
from Alexandre Oliva
PR debug/48866
* df.h (enum debug_temp_where): New.
(dead_debug_init, dead_debug_finish) Declare.
(dead_debug_add, dead_debug_insert_temp): Declare.
(struct dead_debug_use, struct dead_debug): Moved from...
* df-problems.c: ... here.
(df_set_unused_notes_for_mw): Bind debug uses of unused regno
to a debug temp.
(df_create_unused_note): Likewise.
(df_set_dead_notes_for_mw): Move comment where it belongs.
(dead_debug_init): Export.
(dead_debug_reset_uses): New, factored out of...
(dead_debug_finish): ...this. Export.
(dead_debug_reset): Remove.
(dead_debug_add): Export.
(dead_debug_insert_before): Rename to...
(dead_debug_insert_temp): ... this. Add where argument. Export.
Locate stored value for BEFORE_WITH_VALUE. Avoid repeat inserts.
Return insertion count.
(df_note_bb_compute): Adjust.
* dce.c (word_dce_process_block): Adjust dead debug uses.
(dce_process_block): Likewise.
Index: gcc/df.h
===
--- gcc/df.h.orig 2012-04-13 05:18:29.140781640 -0300
+++ gcc/df.h 2012-04-13 07:13:18.0 -0300
@@ -1101,4 +1101,46 @@ extern void union_defs (df_ref, struct w
unsigned int *used, struct web_entry *,
bool (*fun) (struct web_entry *, struct web_entry *));
+/* Debug uses of dead regs. */
+
+/* Node of a linked list of uses of dead REGs in debug insns. */
+struct dead_debug_use
+{
+ df_ref use;
+ struct dead_debug_use *next;
+};
+
+/* Linked list of the above, with a bitmap of the REGs in the
+ list. */
+struct dead_debug
+{
+ struct dead_debug_use *head;
+ bitmap used;
+ bitmap to_rescan;
+};
+
+/* This type controls the behavior of dead_debug_insert_temp WRT
+ UREGNO and INSN. */
+enum debug_temp_where
+ {
+/* Bind a newly-created debug temporary to a REG for UREGNO, and
+ insert the debug insn before INSN. REG is expected to die at
+ INSN. */
+DEBUG_TEMP_BEFORE_WITH_REG = -1,
+/* Bind a newly-created debug temporary to the value INSN stores
+ in REG, and insert the debug insn before INSN. */
+DEBUG_TEMP_BEFORE_WITH_VALUE = 0,
+/* Bind a newly-created debug temporary to a REG for UREGNO, and
+ insert the debug insn after INSN. REG is expected to be set at
+ INSN. */
+DEBUG_TEMP_AFTER_WITH_REG = 1
+ };
+
+extern void dead_debug_init (struct dead_debug *, bitmap);
+extern void dead_debug_finish (struct dead_debug *, bitmap);
+extern void dead_debug_add (struct dead_debug *, df_ref, unsigned int);
+extern int dead_debug_insert_temp (struct dead_debug *,
+ unsigned int uregno, rtx insn,
+ enum debug_temp_where);
+
#endif /* GCC_DF_H */
Index: gcc/df-problems.c
===
--- gcc/df-problems.c.orig 2012-04-13 06:58:42.053258184 -0300
+++ gcc/df-problems.c 2012-04-13 07:29:33.0 -0300
@@ -2886,25 +2886,6 @@ df_whole_mw_reg_unused_p (struct df_mw_h
}
-/* Node of a linked list of uses of dead REGs in debug insns. */
-struct dead_debug_use
-{
- df_ref use;
- struct dead_debug_use *next;
-};
-
-/* Linked list of the above, with a bitmap of the REGs in the
- list. */
-struct dead_debug
-{
- struct dead_debug_use *head;
- bitmap used;
- bitmap to_rescan;
-};
-
-static void dead_debug_reset (struct dead_debug *, unsigned int);
-
-
/* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
based on the bits in LIVE. Do not generate notes for registers in
artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
@@ -2930,7 +2911,7 @@ df_set_unused_notes_for_mw (rtx insn, st
{
unsigned int regno = mws->start_regno;
df_set_note (REG_UNUSED, insn, mws->mw_reg);
- dead_debug_reset (debug, regno);
+ dead_debug_insert_temp (debug, regno, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -2945,7 +2926,7 @@ df_set_unused_notes_for_mw (rtx insn, st
&& !bitmap_bit_p (artificial_uses, r))
{
df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
- dead_debug_reset (debug, r);
+ dead_debug_insert_temp (debug, r, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 2: ", insn, REG_NOTES (insn));
#endif
@@ -3013,12 +2994,12 @@ df_set_dead_notes_for_mw (rtx insn, stru
if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
{
- /* Add a dead note for the entire m
Re: fix left-over debug insns in DCE
> Some more context here: this enables DCE to turn removed insns into
> debug temps when they're useful for debug info. It further improves
> debug info quality when installed along with the patch I just posted for
> PR 48866. Without it, a number of chains of debug temps that lead to a
> real insn that gets deleted end up useless.
> Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu. Ok to install?
OK for mainline, modulo the following nits:
+enum debug_temp_where
+ {
+DEBUG_TEMP_BEFORE_WITH_REG = -1,
+DEBUG_TEMP_BEFORE_WITH_VALUE = 0,
+DEBUG_TEMP_AFTER_WITH_REG = 1
+ };
Could you add a comment for each value?
+void dead_debug_init (struct dead_debug *, bitmap);
+void dead_debug_finish (struct dead_debug *, bitmap);
+void dead_debug_add (struct dead_debug *, df_ref, unsigned int);
+int dead_debug_insert_temp (struct dead_debug *, unsigned int, rtx,
+ enum debug_temp_where);
Missing "extern" for all declarations.
/* If UREGNO is referenced by any entry in DEBUG, emit a debug insn
+ before or after INSN (depending on WHERE), that binds a debug temp
+ to the widest-mode use of UREGNO, if WHERE is _WITH_REG, or
+ _WITH_VALUE otherwise, and replace all uses of UREGNO in DEBUG with
+ uses of the debug temp. INSN must be where UREGNO dies, if WHERE
+ is DEAD_DEBUG_BEFORE_WITH_REG, or where it is set otherwise.
+ Return the number of debug insns emitted. */
I don't understand the "or _WITH_VALUE otherwise" part of the comment.
+ if (where == DEBUG_TEMP_BEFORE_WITH_VALUE)
+{
+ rtx set = single_set (insn);
+ rtx dest, src;
+
+ if (set)
+ {
+ dest = SET_DEST (set);
+ src = SET_SRC (set);
+ if (GET_CODE (src) == CALL)
+ {
+ while (uses)
+ {
+ cur = uses->next;
+ XDELETE (uses);
+ uses = cur;
+ }
+ return 0;
+ }
+ }
+ else
+ set = NULL;
+
+ if (!set)
+ breg = NULL;
+ else if (dest == reg)
+ breg = copy_rtx (src);
+ else if (REG_P (dest))
+ {
+ if (REGNO (dest) != REGNO (reg))
+ breg = NULL;
+ else
+ breg = lowpart_subreg (GET_MODE (reg), copy_rtx (src),
+ GET_MODE (dest));
+ }
+ else if (GET_CODE (dest) == SUBREG)
+ {
+ if (REGNO (SUBREG_REG (dest)) != REGNO (reg))
+ breg = NULL;
+ else if (!subreg_lowpart_p (dest))
+ breg = NULL;
+ else if (REGNO (reg) < FIRST_PSEUDO_REGISTER
+ && (hard_regno_nregs[REGNO (reg)][GET_MODE (reg)]
+ == hard_regno_nregs[REGNO (reg)][GET_MODE (dest)]))
+ breg = NULL;
+ else
+ breg = lowpart_subreg (GET_MODE (reg), copy_rtx (src),
+ GET_MODE (dest));
+ }
+ else
+ breg = NULL;
+
+ if (!breg)
+ {
+ dead_debug_reset_uses (debug, uses);
+ return 0;
+ }
+}
Please add a comment explaining what this is doing.
--
Eric Botcazou
Re: fix left-over debug insns in DCE
On Jun 6, 2011, Alexandre Oliva wrote:
> On Jun 6, 2011, Eric Botcazou wrote:
>>> It might be too late for DF to do anything sensible to preserve the
>>> debug info rather than just throw it away, as your partial approval
>>> suggests.
>> OK, let me think about this a little more.
Ping?
> Here are the remaining bits.
Some more context here: this enables DCE to turn removed insns into
debug temps when they're useful for debug info. It further improves
debug info quality when installed along with the patch I just posted for
PR 48866. Without it, a number of chains of debug temps that lead to a
real insn that gets deleted end up useless.
Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu. Ok to install?
for gcc/ChangeLog
from Alexandre Oliva
PR debug/48866
* df.h (enum debug_temp_where): New.
(dead_debug_init, dead_debug_finish) Declare.
(dead_debug_add, dead_debug_insert_temp): Declare.
(struct dead_debug_use, struct dead_debug): Moved from...
* df-problems.c: ... here.
(df_set_unused_notes_for_mw): Bind debug uses of unused regno
to a debug temp.
(df_create_unused_note): Likewise.
(df_set_dead_notes_for_mw): Move comment where it belongs.
(dead_debug_init): Export.
(dead_debug_reset_uses): New, factored out of...
(dead_debug_finish): ...this. Export.
(dead_debug_reset): Remove.
(dead_debug_add): Export.
(dead_debug_insert_before): Rename to...
(dead_debug_insert_temp): ... this. Add where argument. Export.
Locate stored value for BEFORE_WITH_VALUE. Avoid repeat inserts.
Return insertion count.
(df_note_bb_compute): Adjust.
* dce.c (word_dce_process_block): Adjust dead debug uses.
(dce_process_block): Likewise.
Index: gcc/df.h
===
--- gcc/df.h.orig 2012-03-01 04:26:27.926134403 -0300
+++ gcc/df.h 2012-03-26 11:19:12.300584463 -0300
@@ -1101,4 +1101,35 @@ extern void union_defs (df_ref, struct w
unsigned int *used, struct web_entry *,
bool (*fun) (struct web_entry *, struct web_entry *));
+/* Debug uses of dead regs. */
+
+/* Node of a linked list of uses of dead REGs in debug insns. */
+struct dead_debug_use
+{
+ df_ref use;
+ struct dead_debug_use *next;
+};
+
+/* Linked list of the above, with a bitmap of the REGs in the
+ list. */
+struct dead_debug
+{
+ struct dead_debug_use *head;
+ bitmap used;
+ bitmap to_rescan;
+};
+
+enum debug_temp_where
+ {
+DEBUG_TEMP_BEFORE_WITH_REG = -1,
+DEBUG_TEMP_BEFORE_WITH_VALUE = 0,
+DEBUG_TEMP_AFTER_WITH_REG = 1
+ };
+
+void dead_debug_init (struct dead_debug *, bitmap);
+void dead_debug_finish (struct dead_debug *, bitmap);
+void dead_debug_add (struct dead_debug *, df_ref, unsigned int);
+int dead_debug_insert_temp (struct dead_debug *, unsigned int, rtx,
+ enum debug_temp_where);
+
#endif /* GCC_DF_H */
Index: gcc/df-problems.c
===
--- gcc/df-problems.c.orig 2012-01-30 16:47:05.0 -0200
+++ gcc/df-problems.c 2012-03-26 11:49:17.848542873 -0300
@@ -2886,25 +2886,6 @@ df_whole_mw_reg_unused_p (struct df_mw_h
}
-/* Node of a linked list of uses of dead REGs in debug insns. */
-struct dead_debug_use
-{
- df_ref use;
- struct dead_debug_use *next;
-};
-
-/* Linked list of the above, with a bitmap of the REGs in the
- list. */
-struct dead_debug
-{
- struct dead_debug_use *head;
- bitmap used;
- bitmap to_rescan;
-};
-
-static void dead_debug_reset (struct dead_debug *, unsigned int);
-
-
/* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
based on the bits in LIVE. Do not generate notes for registers in
artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
@@ -2930,7 +2911,7 @@ df_set_unused_notes_for_mw (rtx insn, st
{
unsigned int regno = mws->start_regno;
df_set_note (REG_UNUSED, insn, mws->mw_reg);
- dead_debug_reset (debug, regno);
+ dead_debug_insert_temp (debug, regno, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -2945,7 +2926,7 @@ df_set_unused_notes_for_mw (rtx insn, st
&& !bitmap_bit_p (artificial_uses, r))
{
df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
- dead_debug_reset (debug, r);
+ dead_debug_insert_temp (debug, r, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 2: ", insn, REG_NOTES (insn));
#endif
@@ -3013,12 +2994,12 @@ df_set_dead_notes_for_mw (rtx insn, stru
if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
{
- /* Add a dead note for the entire multi word register. */
if (is_debug)
{
*added_notes_p = true;
return;
}
+ /* Add a dead note for the entire multi word register. */
df_set_note (REG_DEAD, insn, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -3072,7 +3053,7 @@ df_create_u
Re: fix left-over debug insns in DCE
On Jun 6, 2011, Eric Botcazou wrote:
>> It might be too late for DF to do anything sensible to preserve the
>> debug info rather than just throw it away, as your partial approval
>> suggests.
> OK, let me think about this a little more.
>> Indeed, sorry, I misread it.
> Mind installing these bits separately? My understanding is that they are
> independent correctness fixes.
Tested, installed.
Here are the remaining bits.
for gcc/ChangeLog
from Alexandre Oliva
* df.h (enum debug_temp_where): New.
(dead_debug_init, dead_debug_finish) Declare.
(dead_debug_add, dead_debug_insert_temp): Declare.
(struct dead_debug_use, struct dead_debug): Moved from...
* df-problems.c: ... here.
(df_set_unused_notes_for_mw): Bind debug uses of unused regno
to a debug temp.
(df_create_unused_note): Likewise.
(df_set_dead_notes_for_mw): Move comment where it belongs.
(dead_debug_init): Export.
(dead_debug_reset_uses): New, factored out of...
(dead_debug_finish): ...this. Export.
(dead_debug_reset): Remove.
(dead_debug_add): Export.
(dead_debug_insert_before): Rename to...
(dead_debug_insert_temp): ... this. Add where argument. Export.
Locate stored value for BEFORE_WITH_VALUE. Avoid repeat inserts.
Return insertion count.
(df_note_bb_compute): Adjust.
* dce.c (word_dce_process_block): Adjust dead debug uses.
(dce_process_block): Likewise.
Index: gcc/df-problems.c
===
--- gcc/df-problems.c.orig 2011-06-03 11:41:26.507743096 -0300
+++ gcc/df-problems.c 2011-06-06 10:25:17.854030387 -0300
@@ -2842,25 +2842,6 @@ df_whole_mw_reg_unused_p (struct df_mw_h
}
-/* Node of a linked list of uses of dead REGs in debug insns. */
-struct dead_debug_use
-{
- df_ref use;
- struct dead_debug_use *next;
-};
-
-/* Linked list of the above, with a bitmap of the REGs in the
- list. */
-struct dead_debug
-{
- struct dead_debug_use *head;
- bitmap used;
- bitmap to_rescan;
-};
-
-static void dead_debug_reset (struct dead_debug *, unsigned int);
-
-
/* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
based on the bits in LIVE. Do not generate notes for registers in
artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
@@ -2886,7 +2867,7 @@ df_set_unused_notes_for_mw (rtx insn, st
{
unsigned int regno = mws->start_regno;
df_set_note (REG_UNUSED, insn, mws->mw_reg);
- dead_debug_reset (debug, regno);
+ dead_debug_insert_temp (debug, regno, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -2901,7 +2882,7 @@ df_set_unused_notes_for_mw (rtx insn, st
&& !bitmap_bit_p (artificial_uses, r))
{
df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
- dead_debug_reset (debug, r);
+ dead_debug_insert_temp (debug, r, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 2: ", insn, REG_NOTES (insn));
#endif
@@ -2969,12 +2950,12 @@ df_set_dead_notes_for_mw (rtx insn, stru
if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
{
- /* Add a dead note for the entire multi word register. */
if (is_debug)
{
*added_notes_p = true;
return;
}
+ /* Add a dead note for the entire multi word register. */
df_set_note (REG_DEAD, insn, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -3028,7 +3009,7 @@ df_create_unused_note (rtx insn, df_ref
rtx reg = (DF_REF_LOC (def))
? *DF_REF_REAL_LOC (def): DF_REF_REG (def);
df_set_note (REG_UNUSED, insn, reg);
- dead_debug_reset (debug, dregno);
+ dead_debug_insert_temp (debug, dregno, insn, DEBUG_TEMP_AFTER_WITH_REG);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 3: ", insn, REG_NOTES (insn));
#endif
@@ -3039,7 +3020,7 @@ df_create_unused_note (rtx insn, df_ref
/* Initialize DEBUG to an empty list, and clear USED, if given. */
-static inline void
+void
dead_debug_init (struct dead_debug *debug, bitmap used)
{
debug->head = NULL;
@@ -3049,32 +3030,83 @@ dead_debug_init (struct dead_debug *debu
bitmap_clear (used);
}
-/* Reset all debug insns with pending uses. Release the bitmap in it,
- unless it is USED. USED must be the same bitmap passed to
- dead_debug_init. */
-static inline void
-dead_debug_finish (struct dead_debug *debug, bitmap used)
+/* Reset all debug uses in HEAD, and clear DEBUG->to_rescan bits of
+ each reset insn. DEBUG is not otherwise modified. If HEAD is
+ DEBUG->head, DEBUG->head will be set to NULL at the end.
+ Otherwise, entries from DEBUG->head that pertain to reset insns
+ will be removed, and only then rescanned. */
+
+static void
+dead_debug_reset_uses (struct dead_debug *debug, struct dead_debug_use *head)
{
- struct dead_debug_use *head;
- rtx insn = NULL;
+ bool got_head = (debug->head ==
Re: fix left-over debug insns in DCE
On Jun 6, 2011, Eric Botcazou wrote:
>> Indeed, sorry, I misread it.
> Mind installing these bits separately?
Nope. Testing it separately now.
> My understanding is that they are
> independent correctness fixes.
Yeah, I guess they are. They won't make debug info any worse, although
they may remove incorrect debug info, and they won't preserve debug info
that we currently discard, which the other part of the patch retains.
Here's what I'm going to check in if regstrapping succeeds.
for gcc/ChangeLog
from Alexandre Oliva
* dce.c (reset_unmarked_insns_debug_uses): New.
(delete_unmarked_insns): Skip debug insns.
(prescan_insns_for_dce): Likewise.
(rest_of_handle_ud_dce): Reset debug uses of removed sets.
* reg-stack.c (subst_stack_regs_in_debug_insn): Signal when no
active reg can be found.
(subst_all_stack_regs_in_debug_insn): New. Reset debug insn then.
(convert_regs_1): Use it.
Index: gcc/dce.c
===
--- gcc/dce.c.orig 2011-05-26 05:03:08.552989108 -0300
+++ gcc/dce.c 2011-05-27 03:09:07.711388764 -0300
@@ -493,6 +493,43 @@ remove_reg_equal_equiv_notes_for_defs (r
remove_reg_equal_equiv_notes_for_regno (DF_REF_REGNO (*def_rec));
}
+/* Scan all BBs for debug insns and reset those that reference values
+ defined in unmarked insns. */
+
+static void
+reset_unmarked_insns_debug_uses (void)
+{
+ basic_block bb;
+ rtx insn, next;
+
+ FOR_EACH_BB_REVERSE (bb)
+FOR_BB_INSNS_REVERSE_SAFE (bb, insn, next)
+ if (DEBUG_INSN_P (insn))
+ {
+ df_ref *use_rec;
+
+ for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
+ {
+ df_ref use = *use_rec;
+ struct df_link *defs;
+ for (defs = DF_REF_CHAIN (use); defs; defs = defs->next)
+ {
+ rtx insn;
+ if (DF_REF_IS_ARTIFICIAL (defs->ref))
+ continue;
+ insn = DF_REF_INSN (defs->ref);
+ if (!marked_insn_p (insn))
+ break;
+ }
+ if (!defs)
+ continue;
+ /* ??? FIXME could we propagate the values assigned to
+ each of the DEFs? */
+ INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
+ df_insn_rescan_debug_internal (insn);
+ }
+ }
+}
/* Delete every instruction that hasn't been marked. */
@@ -505,7 +542,7 @@ delete_unmarked_insns (void)
FOR_EACH_BB_REVERSE (bb)
FOR_BB_INSNS_REVERSE_SAFE (bb, insn, next)
- if (INSN_P (insn))
+ if (NONDEBUG_INSN_P (insn))
{
/* Always delete no-op moves. */
if (noop_move_p (insn))
@@ -579,7 +616,7 @@ prescan_insns_for_dce (bool fast)
FOR_EACH_BB (bb)
{
FOR_BB_INSNS_REVERSE_SAFE (bb, insn, prev)
- if (INSN_P (insn))
+ if (NONDEBUG_INSN_P (insn))
{
/* Don't mark argument stores now. They will be marked
if needed when the associated CALL is marked. */
@@ -713,6 +750,9 @@ rest_of_handle_ud_dce (void)
}
VEC_free (rtx, heap, worklist);
+ if (MAY_HAVE_DEBUG_INSNS)
+reset_unmarked_insns_debug_uses ();
+
/* Before any insns are deleted, we must remove the chains since
they are not bidirectional. */
df_remove_problem (df_chain);
Index: gcc/reg-stack.c
===
--- gcc/reg-stack.c.orig 2011-05-26 05:03:08.554989112 -0300
+++ gcc/reg-stack.c 2011-05-27 03:11:23.756589358 -0300
@@ -1333,6 +1333,11 @@ subst_stack_regs_in_debug_insn (rtx *loc
return 0;
hard_regno = get_hard_regnum (regstack, *loc);
+
+ /* If we can't find an active register, reset this debug insn. */
+ if (hard_regno == -1)
+return 1;
+
gcc_assert (hard_regno >= FIRST_STACK_REG);
replace_reg (loc, hard_regno);
@@ -1340,6 +1345,23 @@ subst_stack_regs_in_debug_insn (rtx *loc
return -1;
}
+/* Substitute hardware stack regs in debug insn INSN, using stack
+ layout REGSTACK. If we can't find a hardware stack reg for any of
+ the REGs in it, reset the debug insn. */
+
+static void
+subst_all_stack_regs_in_debug_insn (rtx insn, struct stack_def *regstack)
+{
+ int ret = for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
+ subst_stack_regs_in_debug_insn,
+ regstack);
+
+ if (ret == 1)
+INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
+ else
+gcc_checking_assert (ret == 0);
+}
+
/* Substitute new registers in PAT, which is part of INSN. REGSTACK
is the current register layout. Return whether a control flow insn
was deleted in the process. */
@@ -2947,8 +2969,7 @@ convert_regs_1 (basic_block block)
debug_insns_with_starting_stack++;
else
{
- for_each_rtx (&PATTERN (insn), subst_stack_regs_in_debug_insn,
- ®stack);
+ subst_all_stack_regs_in_debug_insn (insn, ®stack);
/* Nothing must ever die at a debug insn. If something
is referenced in it that becomes dead, it should have
@@ -2986,8 +3007,7 @@ convert_regs_1 (basic_block block)
continue;
debug_insns_with_starting_stack--;
- for_each_rtx (&PATTERN (insn), subst_stack_reg
Re: fix left-over debug insns in DCE
> It might be too late for DF to do anything sensible to preserve the > debug info rather than just throw it away, as your partial approval > suggests. OK, let me think about this a little more. > Indeed, sorry, I misread it. Mind installing these bits separately? My understanding is that they are independent correctness fixes. -- Eric Botcazou
Re: fix left-over debug insns in DCE
On Jun 3, 2011, Eric Botcazou wrote: > Does the same logic need to be replicated in all passes that do? On > the other hand, these passes call into DF when they remove insns, so > DF is a central place here. I went over (again?) a number of passes that call delete_insn directly or indirectly. The problem is that, at that point, it's not possible to tell whether: - equivalent DEFs are going to be re-emitted at the same place (say, a split), so debug insns are to be left alone, - whether they're being re-emitted elsewhere (say a move), so *some* debug insns may have to be adjusted - whether DEFs are really being removed, in which case debug temps may have to be emitted; and the uses, adjusted; and other reaching defs also followed by binding of the same debug temps - whether many chained insns are being deleted (say an entire BB, or part of a BB found to be shared between multiple BBs), in which case debug insns may have to be left alone or adjusted depending on the CFG adjustments to be made later Given this analysis, I'm still of the opinion that passes that remove insns that may be used by debug insns ought to decide on their own how to make the adjustments (or not), as they currently do, but that we should have better shared infrastructure to propagate defs-to-be-removed, which is currently done in an ad hoc manner in several passes. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: fix left-over debug insns in DCE
On Mon, Jun 06, 2011 at 02:32:29AM -0300, Alexandre Oliva wrote: > Those that remove sets whose DESTs may still be receved by debug insns > ought to adjust debug insns, yeah. I think (hope) we have them all > covered. Do you know of any we missed? delete_trivially_dead_insns is already covered, fast DCE is what I've encountered to be missing in the past (see PR48886), but I guess your patch covers both fast DCE and ud DCE. Jakub
Re: fix left-over debug insns in DCE
On Jun 3, 2011, Eric Botcazou wrote: >> Hmm... Maybe it could, I'm not sure. The problem is that DCE removes >> insns, and then DF associates remaining uses in debug insns to earlier >> DEFs. Adjusting debug insns in DCE is right per the VTA design motto: >> decide as if debug insns weren't there, adjust them as you would adjust >> non-debug insns. This code borrowed from DF into DCE is the “adjust” >> bit. > But DCE isn't the only pass that removes insns. Yup. > Does the same logic need to be replicated in all passes that do? Those that remove sets whose DESTs may still be receved by debug insns ought to adjust debug insns, yeah. I think (hope) we have them all covered. Do you know of any we missed? > On the other hand, these passes call into DF when they remove insns, > so DF is a central place here. It might be too late for DF to do anything sensible to preserve the debug info rather than just throw it away, as your partial approval suggests. >> Err... These depend on the interface changes of functions defined >> within DF to work. > No, they don't, I can compile them independently. Indeed, sorry, I misread it. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: fix left-over debug insns in DCE
> Hmm... Maybe it could, I'm not sure. The problem is that DCE removes > insns, and then DF associates remaining uses in debug insns to earlier > DEFs. Adjusting debug insns in DCE is right per the VTA design motto: > decide as if debug insns weren't there, adjust them as you would adjust > non-debug insns. This code borrowed from DF into DCE is the “adjust” > bit. But DCE isn't the only pass that removes insns. Does the same logic need to be replicated in all passes that do? On the other hand, these passes call into DF when they remove insns, so DF is a central place here. > Err... These depend on the interface changes of functions defined > within DF to work. No, they don't, I can compile them independently. -- Eric Botcazou
Re: fix left-over debug insns in DCE
On Jun 2, 2011, Eric Botcazou wrote: > Why can't the problem be addressed purely within DF? Hmm... Maybe it could, I'm not sure. The problem is that DCE removes insns, and then DF associates remaining uses in debug insns to earlier DEFs. Adjusting debug insns in DCE is right per the VTA design motto: decide as if debug insns weren't there, adjust them as you would adjust non-debug insns. This code borrowed from DF into DCE is the “adjust” bit. > Starting to spill the DF > logic to individual RTL passes doesn't look very appealing to me. Propagation of uses isn't DF-specific material, it just so happened that it offered an adequate interface. Other passes already have their own propagation machinery, but it didn't look quite as suitable. >> This is the patch I ended up with. Regstrapped on x86_64-linux-gnu and >> i686-linux-gnu. Ok to install? > OK for the usual debug insn bookkeeping, i.e. Err... These depend on the interface changes of functions defined within DF to work. Should they perhaps be moved out of DF-specific files? -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: fix left-over debug insns in DCE
> One of the issues was that DCE removed an insn that set a REG in a > certain mode, without adjusting a debug use of that REG. This was in > libstdc++, but I failed to take note of the affected file. DF later > attached that debug use to another SET to the same REG in a different, > incompatible mode. When that one was found to be dead by DF, we ended > up ICEing as we attempted to emit the invalid SUBREGs. > > I reused some of the infrastructure to propagate dead DEFs into debug > uses in DF to get DCE to emit debug temps and adjust debug uses as well, > fixing this issue. While at that, I improved the handling of unused > DEFs in DF, that previously resulted in loss of debug information, so as > to retain it as much as possible. Why can't the problem be addressed purely within DF? Starting to spill the DF logic to individual RTL passes doesn't look very appealing to me. > This is the patch I ended up with. Regstrapped on x86_64-linux-gnu and > i686-linux-gnu. Ok to install? OK for the usual debug insn bookkeeping, i.e. * dce.c (reset_unmarked_insns_debug_uses): New. (delete_unmarked_insns): Skip debug insns. (prescan_insns_for_dce): Likewise. (rest_of_handle_ud_dce): Propagate debug uses. * reg-stack.c (subst_stack_regs_in_debug_insn): Signal when no active reg can be found. (subst_all_stack_regs_in_debug_insn): New. Reset debug insn then. (convert_regs_1): Use it. The rest needs further discussing IMO. -- Eric Botcazou
