[PATCH] Avoid extending reg lifetime when cprop_hardreg propagates into DEBUG_INSNs (PR debug/43161)
Jakub Jelinek
jakub@redhat.com
Wed Feb 24 20:26:00 GMT 2010
Hi!
The following patch fixes
FAIL: gcc.dg/guality/vla-1.c -O2 line 24 sizeof (a) == 17 * sizeof (short)
FAIL: gcc.dg/guality/vla-1.c -O3 -fomit-frame-pointer line 24 sizeof (a) == 17 * sizeof (short)
FAIL: gcc.dg/guality/vla-1.c -O3 -g line 24 sizeof (a) == 17 * sizeof (short)
FAIL: gcc.dg/guality/vla-1.c -Os line 24 sizeof (a) == 17 * sizeof (short)
failures on x86_64-linux. The problem there is that the cprop_hardreg pass
propagates even into DEBUG_INSNs (which makes some sense, we want the
regs used in DEBUG_INSN match what the real insn use as much as possible,
otherwise we risk the DEBUG_INSNs need to be reset), and in some cases as on
this testcase enlarges lifetime of the hard reg by that (which results in
the next pass that computes df note problem to drop those DEBUG_INSNs on the
floor, as extending lifetime of regs in DEBUG_INSNs is forbidden).
As this pass is a forward iteration through bb's, we don't know yet whether
a subsequent real insn will extend the lifetime of the hard reg to make the
DEBUG_INSN change desirable (or if it is already live across the debug
insn), so this patch solves it by not applying changes against DEBUG_INSNs
right away, but instead queueing the changes. If we later on find out the
new hard reg is live across the DEBUG_INSN, we just apply the changes,
if we reach end of the bb, we keep the changes around and when the whole
pass is over, df_analyze and apply changes to DEBUG_INSNs where the hard
regs are still live at the end of those bbs.
I wasn't 100% sure whether calling df_analyze () (without FAST_DCE in flags)
just when -fvar-tracking-assignments is used wouldn't create -fcompare-debug
differences, but I've bootstrapped both this version of the patch and the
one in bugzilla which called df_analyze unconditionally, and both
bootstrapped/regtested fine on x86_64-linux and i686-linux and additionally
had the same make check RUNTESTFLAGS=--target_board=unix/-fcompare-debug
results, so I hope we are ok.
Ok for trunk?
2010-02-24 Jakub Jelinek <jakub@redhat.com>
PR debug/43161
* regcprop.c (struct queued_debug_insn_change): New type.
(struct value_data_entry): Add debug_insn_changes field.
(struct value_data): Add n_debug_insn_changes field.
(free_debug_insn_changes, apply_debug_insn_changes,
cprop_find_used_regs_1, cprop_find_used_regs): New functions.
(kill_value_one_regno): Call free_debug_insn_changes if needed.
(init_value_data): Clear debug_insn_changes and n_debug_insn_changes
fields.
(replace_oldest_value_reg): Don't change DEBUG_INSNs, instead queue
changes for them.
(copyprop_hardreg_forward_1): Don't call apply_change_group for
DEBUG_INSNs. For a real insn, if there are queued DEBUG_INSN
changes, call cprop_find_used_regs via note_stores.
(copyprop_hardreg_forward): When copying vd from predecessor
which has any queued DEBUG_INSN changes, make sure the pointers are
cleared. At the end call df_analyze and then if there are any
DEBUG_INSN changes queued at the end of some basic block for still
live registers, apply them.
(pass_cprop_hardreg): Set TODO_df_finish in todo_flags_finish.
--- gcc/regcprop.c.jj 2010-02-23 18:09:24.000000000 +0100
+++ gcc/regcprop.c 2010-02-24 13:10:37.000000000 +0100
@@ -46,6 +46,18 @@
up some silly register allocation decisions made by reload. This
code may be obsoleted by a new register allocator. */
+/* DEBUG_INSNs aren't changed right away, as doing so might extend the
+ lifetime of a register and get the DEBUG_INSN subsequently reset.
+ So they are queued instead, and updated only when the register is
+ used in some subsequent real insn before it is set. */
+struct queued_debug_insn_change
+{
+ struct queued_debug_insn_change *next;
+ rtx insn;
+ rtx *loc;
+ rtx new_rtx;
+};
+
/* For each register, we have a list of registers that contain the same
value. The OLDEST_REGNO field points to the head of the list, and
the NEXT_REGNO field runs through the list. The MODE field indicates
@@ -57,12 +69,14 @@ struct value_data_entry
enum machine_mode mode;
unsigned int oldest_regno;
unsigned int next_regno;
+ struct queued_debug_insn_change *debug_insn_changes;
};
struct value_data
{
struct value_data_entry e[FIRST_PSEUDO_REGISTER];
unsigned int max_value_regs;
+ unsigned int n_debug_insn_changes;
};
static void kill_value_one_regno (unsigned, struct value_data *);
@@ -91,6 +105,22 @@ extern void debug_value_data (struct val
static void validate_value_data (struct value_data *);
#endif
+/* Free all queued updates for DEBUG_INSNs that change some reg to
+ register REGNO. */
+
+static void
+free_debug_insn_changes (struct value_data *vd, unsigned int regno)
+{
+ struct queued_debug_insn_change *cur, *next;
+ for (cur = vd->e[regno].debug_insn_changes; cur; cur = next)
+ {
+ next = cur->next;
+ --vd->n_debug_insn_changes;
+ XDELETE (cur);
+ }
+ vd->e[regno].debug_insn_changes = NULL;
+}
+
/* Kill register REGNO. This involves removing it from any value
lists, and resetting the value mode to VOIDmode. This is only a
helper function; it does not handle any hard registers overlapping
@@ -118,6 +148,8 @@ kill_value_one_regno (unsigned int regno
vd->e[regno].mode = VOIDmode;
vd->e[regno].oldest_regno = regno;
vd->e[regno].next_regno = INVALID_REGNUM;
+ if (vd->e[regno].debug_insn_changes)
+ free_debug_insn_changes (vd, regno);
#ifdef ENABLE_CHECKING
validate_value_data (vd);
@@ -204,8 +236,10 @@ init_value_data (struct value_data *vd)
vd->e[i].mode = VOIDmode;
vd->e[i].oldest_regno = i;
vd->e[i].next_regno = INVALID_REGNUM;
+ vd->e[i].debug_insn_changes = NULL;
}
vd->max_value_regs = 0;
+ vd->n_debug_insn_changes = 0;
}
/* Called through note_stores. If X is clobbered, kill its value. */
@@ -446,6 +480,23 @@ replace_oldest_value_reg (rtx *loc, enum
rtx new_rtx = find_oldest_value_reg (cl, *loc, vd);
if (new_rtx)
{
+ if (DEBUG_INSN_P (insn))
+ {
+ struct queued_debug_insn_change *change;
+
+ if (dump_file)
+ fprintf (dump_file, "debug_insn %u: queued replacing reg %u with %u\n",
+ INSN_UID (insn), REGNO (*loc), REGNO (new_rtx));
+
+ change = XNEW (struct queued_debug_insn_change);
+ change->next = vd->e[REGNO (new_rtx)].debug_insn_changes;
+ change->insn = insn;
+ change->loc = loc;
+ change->new_rtx = new_rtx;
+ vd->e[REGNO (new_rtx)].debug_insn_changes = change;
+ ++vd->n_debug_insn_changes;
+ return true;
+ }
if (dump_file)
fprintf (dump_file, "insn %u: replaced reg %u with %u\n",
INSN_UID (insn), REGNO (*loc), REGNO (new_rtx));
@@ -622,6 +673,58 @@ replace_oldest_value_mem (rtx x, rtx ins
GET_MODE (x), insn, vd);
}
+/* Apply all queued updates for DEBUG_INSNs that change some reg to
+ register REGNO. */
+
+static void
+apply_debug_insn_changes (struct value_data *vd, unsigned int regno)
+{
+ struct queued_debug_insn_change *change;
+ rtx last_insn = vd->e[regno].debug_insn_changes->insn;
+
+ for (change = vd->e[regno].debug_insn_changes;
+ change;
+ change = change->next)
+ {
+ if (last_insn != change->insn)
+ {
+ apply_change_group ();
+ last_insn = change->insn;
+ }
+ validate_change (change->insn, change->loc, change->new_rtx, 1);
+ }
+ apply_change_group ();
+}
+
+/* Called via for_each_rtx, for all used registers in a real
+ insn apply DEBUG_INSN changes that change registers to the
+ used register. */
+
+static int
+cprop_find_used_regs_1 (rtx *loc, void *data)
+{
+ if (REG_P (*loc))
+ {
+ struct value_data *vd = (struct value_data *) data;
+ if (vd->e[REGNO (*loc)].debug_insn_changes)
+ {
+ apply_debug_insn_changes (vd, REGNO (*loc));
+ free_debug_insn_changes (vd, REGNO (*loc));
+ }
+ }
+ return 0;
+}
+
+/* Called via note_uses, for all used registers in a real insn
+ apply DEBUG_INSN changes that change registers to the used
+ registers. */
+
+static void
+cprop_find_used_regs (rtx *loc, void *vd)
+{
+ for_each_rtx (loc, cprop_find_used_regs_1, vd);
+}
+
/* Perform the forward copy propagation on basic block BB. */
static bool
@@ -643,15 +746,10 @@ copyprop_hardreg_forward_1 (basic_block
if (DEBUG_INSN_P (insn))
{
rtx loc = INSN_VAR_LOCATION_LOC (insn);
- if (!VAR_LOC_UNKNOWN_P (loc)
- && replace_oldest_value_addr (&INSN_VAR_LOCATION_LOC (insn),
- ALL_REGS, GET_MODE (loc),
- insn, vd))
- {
- changed = apply_change_group ();
- gcc_assert (changed);
- anything_changed = true;
- }
+ if (!VAR_LOC_UNKNOWN_P (loc))
+ replace_oldest_value_addr (&INSN_VAR_LOCATION_LOC (insn),
+ ALL_REGS, GET_MODE (loc),
+ insn, vd);
}
if (insn == BB_END (bb))
@@ -684,6 +782,10 @@ copyprop_hardreg_forward_1 (basic_block
recog_data.operand_type[i] = OP_INOUT;
}
+ /* Apply changes to earlier DEBUG_INSNs if possible. */
+ if (vd->n_debug_insn_changes)
+ note_uses (&PATTERN (insn), cprop_find_used_regs, vd);
+
/* For each earlyclobber operand, zap the value data. */
for (i = 0; i < n_ops; i++)
if (recog_op_alt[i][alt].earlyclobber)
@@ -871,6 +973,7 @@ copyprop_hardreg_forward (void)
struct value_data *all_vd;
basic_block bb;
sbitmap visited;
+ bool analyze_called = false;
all_vd = XNEWVEC (struct value_data, last_basic_block);
@@ -888,13 +991,54 @@ copyprop_hardreg_forward (void)
if (single_pred_p (bb)
&& TEST_BIT (visited, single_pred (bb)->index)
&& ! (single_pred_edge (bb)->flags & (EDGE_ABNORMAL_CALL | EDGE_EH)))
- all_vd[bb->index] = all_vd[single_pred (bb)->index];
+ {
+ all_vd[bb->index] = all_vd[single_pred (bb)->index];
+ if (all_vd[bb->index].n_debug_insn_changes)
+ {
+ unsigned int regno;
+
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+ {
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+ {
+ all_vd[bb->index].e[regno].debug_insn_changes = NULL;
+ if (--all_vd[bb->index].n_debug_insn_changes == 0)
+ break;
+ }
+ }
+ }
+ }
else
init_value_data (all_vd + bb->index);
copyprop_hardreg_forward_1 (bb, all_vd + bb->index);
}
+ if (MAY_HAVE_DEBUG_STMTS)
+ FOR_EACH_BB (bb)
+ if (TEST_BIT (visited, bb->index)
+ && all_vd[bb->index].n_debug_insn_changes)
+ {
+ unsigned int regno;
+ bitmap live;
+
+ if (!analyze_called)
+ {
+ df_analyze ();
+ analyze_called = true;
+ }
+ live = df_get_live_out (bb);
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+ {
+ if (REGNO_REG_SET_P (live, regno))
+ apply_debug_insn_changes (all_vd + bb->index, regno);
+ free_debug_insn_changes (all_vd + bb->index, regno);
+ if (all_vd[bb->index].n_debug_insn_changes == 0)
+ break;
+ }
+ }
+
sbitmap_free (visited);
free (all_vd);
return 0;
@@ -1026,6 +1170,7 @@ struct rtl_opt_pass pass_cprop_hardreg =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func | TODO_verify_rtl_sharing /* todo_flags_finish */
+ TODO_dump_func | TODO_df_finish
+ | TODO_verify_rtl_sharing /* todo_flags_finish */
}
};
Jakub
More information about the Gcc-patches
mailing list