This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RFC/CFT: Patch to fix the PowerPC var-tracking.c failure
- From: Richard Sandiford <rsandifo at nildram dot co dot uk>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 19 Sep 2007 23:31:39 +0100
- Subject: RFC/CFT: Patch to fix the PowerPC var-tracking.c failure
This message is a CFT and RFC on the patch below, which restores the
DSE "access_size <= UNITS_PER_WORD" condition and appears to fix the
var-tracking.c bug that the condition exposed on PowerPC.
The idea is to use PSEUDO_REGNO_MODE (ORIGINAL_REGNO ...) to detect
cases where a register was a paradoxical subreg of a psuedo before
reload. In that case, the REG_ATTRS of the post-reload register pretend
that the whole register contains useful information, whereas in reality
only the original inner register did. We can then create a
representation of the original inner register with approriately-narrowed
REG_ATTRS. This is the right behaviour for MO_USE, MO_SET and MO_COPY
-- all of which are associated with locations that have trackable
attributes -- but not for MO_USE_NO_VAR and MO_CLOBBER, which record
attributeless uses or clobbers of the whole register.
Adjusting the rtx of a MO_SET and MO_COPY like this means that we
need to adjust any useful SET_SRC in the same way. The current code
goes to great lengths to try to find the original SET_SRC (sometimes
considering COND_EXECs and sometimes not; I'm not sure if that's
deliberate) but it's more convenient to create the new SET_SRC at the
same time as the new SET_DEST. The patch therefore records a SET rather
than a destination if both the SET_DEST and SET_SRC are useful.
I've bootstrapped & regression-tested this patch on x86_64-linux-gnu.
libmudflap.cth/pass40-frag.c started failing for me, but I think that's
just a timeout issue; the test does seem to be running correctly.
I'm not sure if the test is truly slower or not; I'll try to find out.
However, given that the original DSE patch didn't show up any problems
on x86_64-linux-gnu, I'm not sure how much that testing counts for.
It'd be great if someone could test it on other bootstrappable targets,
particularly PowerPC.
I've tried to test this on gdb with both normal options and -O. There
were no differences, but the -O results are so bad that I'm not sure it
means much.
Comments?
Richard
gcc/
* dse.c (find_shift_sequence): Reinstate "<= UNITS_PER_WORD" condition.
* var-tracking.c (micro_operation_def): Update comment on u.loc.
(mode_for_reg_attrs, var_lowpart): New functions.
(add_uses): Consider recording a lowpart of LOC for MO_USE.
(add_stores): Likewise MO_SET and MO_COPY. If the source of a set
or copy is known, set LOC to the SET that performs the set, instead
of the destination.
(find_src_status, find_src_set_src): Remove LOC parameter.
Replace INSN with the source value.
(compute_bb_dataflow, emit_notes_in_bb): Check for a SET u.loc when
handling MO_SET and MO_COPY. Update the calls to find_src_status
and find_src_set_src.
Index: gcc/dse.c
===================================================================
--- gcc/dse.c 2007-09-19 23:04:26.000000000 +0100
+++ gcc/dse.c 2007-09-19 23:04:27.000000000 +0100
@@ -1408,7 +1408,7 @@ find_shift_sequence (rtx read_reg,
justify the value we want to read but is available in one insn on
the machine. */
- for (; access_size < UNITS_PER_WORD; access_size *= 2)
+ for (; access_size <= UNITS_PER_WORD; access_size *= 2)
{
rtx target, new_reg, shift_seq, insn;
enum machine_mode new_mode;
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c 2007-09-19 22:33:35.000000000 +0100
+++ gcc/var-tracking.c 2007-09-19 23:18:11.000000000 +0100
@@ -134,7 +134,9 @@ typedef struct micro_operation_def
enum micro_operation_type type;
union {
- /* Location. */
+ /* Location. For MO_SET and MO_COPY, this is the SET that performs
+ the assignment, if known, otherwise it is the target of the
+ assignment. */
rtx loc;
/* Stack adjustment. */
@@ -1672,6 +1674,54 @@ same_variable_part_p (rtx loc, tree expr
return (expr == expr2 && offset == offset2);
}
+/* REG is a register we want to track. If not all of REG contains useful
+ information, return the mode of the lowpart that does contain useful
+ information, otherwise return the mode of REG.
+
+ If REG was a paradoxical subreg, its REG_ATTRS will describe the
+ whole subreg, but only the old inner part is really relevant. */
+
+static enum machine_mode
+mode_for_reg_attrs (rtx reg)
+{
+ enum machine_mode mode;
+
+ mode = GET_MODE (reg);
+ if (!HARD_REGISTER_NUM_P (ORIGINAL_REGNO (reg)))
+ {
+ enum machine_mode pseudo_mode;
+
+ pseudo_mode = PSEUDO_REGNO_MODE (ORIGINAL_REGNO (reg));
+ if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (pseudo_mode))
+ mode = pseudo_mode;
+ }
+ return mode;
+}
+
+/* Return the MODE lowpart of LOC, or null if LOC is not something we
+ want to track. When returning nonnull, make sure that the attributes
+ on the returned value are updated. */
+
+static rtx
+var_lowpart (enum machine_mode mode, rtx loc)
+{
+ unsigned int offset, regno;
+
+ if (!REG_P (loc) && !MEM_P (loc))
+ return NULL;
+
+ if (GET_MODE (loc) == mode)
+ return loc;
+
+ offset = subreg_lowpart_offset (mode, GET_MODE (loc));
+
+ if (MEM_P (loc))
+ return adjust_address_nv (loc, mode, offset);
+
+ regno = REGNO (loc) + subreg_regno_offset (REGNO (loc), GET_MODE (loc),
+ offset, mode);
+ return gen_rtx_REG_offset (loc, mode, regno, offset);
+}
/* Count uses (register and memory references) LOC which will be tracked.
INSN is instruction which the LOC is part of. */
@@ -1724,9 +1774,16 @@ add_uses (rtx *loc, void *insn)
basic_block bb = BLOCK_FOR_INSN ((rtx) insn);
micro_operation *mo = VTI (bb)->mos + VTI (bb)->n_mos++;
- mo->type = ((REG_EXPR (*loc) && track_expr_p (REG_EXPR (*loc)))
- ? MO_USE : MO_USE_NO_VAR);
- mo->u.loc = *loc;
+ if (REG_EXPR (*loc) && track_expr_p (REG_EXPR (*loc)))
+ {
+ mo->type = MO_USE;
+ mo->u.loc = var_lowpart (mode_for_reg_attrs (*loc), *loc);
+ }
+ else
+ {
+ mo->type = MO_USE_NO_VAR;
+ mo->u.loc = *loc;
+ }
mo->insn = (rtx) insn;
}
else if (MEM_P (*loc)
@@ -1767,16 +1824,35 @@ add_stores (rtx loc, const_rtx expr, voi
if (GET_CODE (expr) == CLOBBER
|| ! REG_EXPR (loc)
|| ! track_expr_p (REG_EXPR (loc)))
- mo->type = MO_CLOBBER;
- else if (GET_CODE (expr) == SET
- && SET_DEST (expr) == loc
- && same_variable_part_p (SET_SRC (expr),
- REG_EXPR (loc),
- REG_OFFSET (loc)))
- mo->type = MO_COPY;
+ {
+ mo->type = MO_CLOBBER;
+ mo->u.loc = loc;
+ }
else
- mo->type = MO_SET;
- mo->u.loc = loc;
+ {
+ enum machine_mode mode = mode_for_reg_attrs (loc);
+ rtx src = NULL;
+
+ if (GET_CODE (expr) == SET && SET_DEST (expr) == loc)
+ src = var_lowpart (mode, SET_SRC (expr));
+ loc = var_lowpart (mode, loc);
+
+ if (src == NULL)
+ {
+ mo->type = MO_SET;
+ mo->u.loc = loc;
+ }
+ else
+ {
+ if (SET_SRC (expr) != src)
+ expr = gen_rtx_SET (VOIDmode, loc, src);
+ if (same_variable_part_p (src, REG_EXPR (loc), REG_OFFSET (loc)))
+ mo->type = MO_COPY;
+ else
+ mo->type = MO_SET;
+ mo->u.loc = CONST_CAST_RTX (expr);
+ }
+ }
mo->insn = (rtx) insn;
}
else if (MEM_P (loc)
@@ -1787,49 +1863,41 @@ add_stores (rtx loc, const_rtx expr, voi
micro_operation *mo = VTI (bb)->mos + VTI (bb)->n_mos++;
if (GET_CODE (expr) == CLOBBER)
- mo->type = MO_CLOBBER;
+ {
+ mo->type = MO_CLOBBER;
+ mo->u.loc = loc;
+ }
else if (GET_CODE (expr) == SET
&& SET_DEST (expr) == loc
- && same_variable_part_p (SET_SRC (expr),
- MEM_EXPR (loc),
- MEM_OFFSET (loc)
- ? INTVAL (MEM_OFFSET (loc)) : 0))
- mo->type = MO_COPY;
+ && var_lowpart (GET_MODE (loc), SET_SRC (expr)))
+ {
+ if (same_variable_part_p (SET_SRC (expr),
+ MEM_EXPR (loc),
+ MEM_OFFSET (loc)
+ ? INTVAL (MEM_OFFSET (loc)) : 0))
+ mo->type = MO_COPY;
+ else
+ mo->type = MO_SET;
+ mo->u.loc = CONST_CAST_RTX (expr);
+ }
else
- mo->type = MO_SET;
- mo->u.loc = loc;
+ {
+ mo->type = MO_SET;
+ mo->u.loc = loc;
+ }
mo->insn = (rtx) insn;
}
}
static enum var_init_status
-find_src_status (dataflow_set *in, rtx loc, rtx insn)
+find_src_status (dataflow_set *in, rtx src)
{
- rtx src = NULL_RTX;
- rtx pattern;
tree decl = NULL_TREE;
enum var_init_status status = VAR_INIT_STATUS_UNINITIALIZED;
if (! flag_var_tracking_uninit)
status = VAR_INIT_STATUS_INITIALIZED;
- pattern = PATTERN (insn);
-
- if (GET_CODE (pattern) == COND_EXEC)
- pattern = COND_EXEC_CODE (pattern);
-
- if (GET_CODE (pattern) == SET)
- src = SET_SRC (pattern);
- else if (GET_CODE (pattern) == PARALLEL
- || GET_CODE (pattern) == SEQUENCE)
- {
- int i;
- for (i = XVECLEN (pattern, 0) - 1; i >= 0; i--)
- if (GET_CODE (XVECEXP (pattern, 0, i)) == SET
- && SET_DEST (XVECEXP (pattern, 0, i)) == loc)
- src = SET_SRC (XVECEXP (pattern, 0, i));
- }
-
if (src && REG_P (src))
decl = var_debug_decl (REG_EXPR (src));
else if (src && MEM_P (src))
@@ -1841,39 +1909,21 @@ find_src_status (dataflow_set *in, rtx l
return status;
}
-/* LOC is the destination the variable is being copied to. INSN
- contains the copy instruction. SET is the dataflow set containing
- the variable in LOC. */
+/* SRC is the source of an assignment. Use SET to try to find what
+ was ultimately assigned to SRC. Return that value if known,
+ otherwise return SRC itself. */
static rtx
-find_src_set_src (dataflow_set *set, rtx loc, rtx insn)
+find_src_set_src (dataflow_set *set, rtx src)
{
tree decl = NULL_TREE; /* The variable being copied around. */
- rtx src = NULL_RTX; /* The location "decl" is being copied from. */
rtx set_src = NULL_RTX; /* The value for "decl" stored in "src". */
- rtx pattern;
void **slot;
variable var;
location_chain nextp;
int i;
bool found;
-
- pattern = PATTERN (insn);
- if (GET_CODE (pattern) == COND_EXEC)
- pattern = COND_EXEC_CODE (pattern);
-
- if (GET_CODE (pattern) == SET)
- src = SET_SRC (pattern);
- else if (GET_CODE (pattern) == PARALLEL
- || GET_CODE (pattern) == SEQUENCE)
- {
- for (i = XVECLEN (pattern, 0) - 1; i >= 0; i--)
- if (GET_CODE (XVECEXP (pattern, 0, i)) == SET
- && SET_DEST (XVECEXP (pattern, 0, i)) == loc)
- src = SET_SRC (XVECEXP (pattern, 0, i));
- }
-
if (src && REG_P (src))
decl = var_debug_decl (REG_EXPR (src));
else if (src && MEM_P (src))
@@ -1947,19 +1997,12 @@ compute_bb_dataflow (basic_block bb)
case MO_SET:
{
rtx loc = VTI (bb)->mos[i].u.loc;
- rtx set_src = NULL;
- rtx insn = VTI (bb)->mos[i].insn;
+ rtx set_src = NULL;
- if (GET_CODE (PATTERN (insn)) == SET)
- set_src = SET_SRC (PATTERN (insn));
- else if (GET_CODE (PATTERN (insn)) == PARALLEL
- || GET_CODE (PATTERN (insn)) == SEQUENCE)
- {
- int j;
- for (j = XVECLEN (PATTERN (insn), 0) - 1; j >= 0; j--)
- if (GET_CODE (XVECEXP (PATTERN (insn), 0, j)) == SET
- && SET_DEST (XVECEXP (PATTERN (insn), 0, j)) == loc)
- set_src = SET_SRC (XVECEXP (PATTERN (insn), 0, j));
+ if (GET_CODE (loc) == SET)
+ {
+ set_src = SET_SRC (loc);
+ loc = SET_DEST (loc);
}
if (REG_P (loc))
@@ -1975,17 +2018,23 @@ compute_bb_dataflow (basic_block bb)
{
rtx loc = VTI (bb)->mos[i].u.loc;
enum var_init_status src_status;
- rtx set_src;
+ rtx set_src = NULL;
+
+ if (GET_CODE (loc) == SET)
+ {
+ set_src = SET_SRC (loc);
+ loc = SET_DEST (loc);
+ }
if (! flag_var_tracking_uninit)
src_status = VAR_INIT_STATUS_INITIALIZED;
else
- src_status = find_src_status (in, loc, VTI (bb)->mos[i].insn);
+ src_status = find_src_status (in, set_src);
if (src_status == VAR_INIT_STATUS_UNKNOWN)
- src_status = find_src_status (out, loc, VTI (bb)->mos[i].insn);
+ src_status = find_src_status (out, set_src);
- set_src = find_src_set_src (in, loc, VTI (bb)->mos[i].insn);
+ set_src = find_src_set_src (in, set_src);
if (REG_P (loc))
var_reg_delete_and_set (out, loc, false, src_status, set_src);
@@ -2883,18 +2932,12 @@ emit_notes_in_bb (basic_block bb)
case MO_SET:
{
rtx loc = VTI (bb)->mos[i].u.loc;
- rtx set_src = NULL;
+ rtx set_src = NULL;
- if (GET_CODE (PATTERN (insn)) == SET)
- set_src = SET_SRC (PATTERN (insn));
- else if (GET_CODE (PATTERN (insn)) == PARALLEL
- || GET_CODE (PATTERN (insn)) == SEQUENCE)
- {
- int j;
- for (j = XVECLEN (PATTERN (insn), 0) - 1; j >= 0; j--)
- if (GET_CODE (XVECEXP (PATTERN (insn), 0, j)) == SET
- && SET_DEST (XVECEXP (PATTERN (insn), 0, j)) == loc)
- set_src = SET_SRC (XVECEXP (PATTERN (insn), 0, j));
+ if (GET_CODE (loc) == SET)
+ {
+ set_src = SET_SRC (loc);
+ loc = SET_DEST (loc);
}
if (REG_P (loc))
@@ -2912,10 +2955,16 @@ emit_notes_in_bb (basic_block bb)
{
rtx loc = VTI (bb)->mos[i].u.loc;
enum var_init_status src_status;
- rtx set_src;
+ rtx set_src = NULL;
+
+ if (GET_CODE (loc) == SET)
+ {
+ set_src = SET_SRC (loc);
+ loc = SET_DEST (loc);
+ }
- src_status = find_src_status (&set, loc, VTI (bb)->mos[i].insn);
- set_src = find_src_set_src (&set, loc, VTI (bb)->mos[i].insn);
+ src_status = find_src_status (&set, set_src);
+ set_src = find_src_set_src (&set, set_src);
if (REG_P (loc))
var_reg_delete_and_set (&set, loc, false, src_status, set_src);