This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFC/CFT: Patch to fix the PowerPC var-tracking.c failure
- From: Richard Sandiford <rsandifo at nildram dot co dot uk>
- To: Eric Botcazou <ebotcazou at libertysurf dot fr>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 12 Oct 2007 17:56:10 +0100
- Subject: Re: RFC/CFT: Patch to fix the PowerPC var-tracking.c failure
- References: <876426nv78.fsf@firetop.home> <200710111608.53370.ebotcazou@libertysurf.fr>
Hi Eric,
Thanks for the review.
Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> * 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.
>
> OK, modulo
>
>> @@ -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;
>> }
>> }
>
> Please use the same structure if the MEM case as in the REG case:
>
> else
> {
> rtx src = NULL;
>
> if (GET_CODE (expr) == SET && SET_DEST (expr) == loc)
> src = var_lowpart (GET_MODE (loc), SET_SRC (expr));
>
> if (src == NULL)
> {
> mo->type = MO_SET;
> mo->u.loc = loc;
> }
> else
> {
> 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);
> }
OK, thanks. (And FWIW, I agree that's an improvement.)
Here's what I checked in after boostrapping & regression-testing on
x86_64-linux-gnu, and after regression-testing on mipsisa32-elf.
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-10-11 22:51:57.000000000 +0100
+++ gcc/dse.c 2007-10-11 22:51:59.000000000 +0100
@@ -1415,7 +1415,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-10-11 22:51:57.000000000 +0100
+++ gcc/var-tracking.c 2007-10-11 22:55:39.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,47 @@ 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;
- else if (GET_CODE (expr) == SET
- && SET_DEST (expr) == loc
- && same_variable_part_p (SET_SRC (expr),
+ {
+ mo->type = MO_CLOBBER;
+ mo->u.loc = loc;
+ }
+ else
+ {
+ rtx src = NULL;
+
+ if (GET_CODE (expr) == SET && SET_DEST (expr) == loc)
+ src = var_lowpart (GET_MODE (loc), SET_SRC (expr));
+
+ if (src == NULL)
+ {
+ mo->type = MO_SET;
+ mo->u.loc = loc;
+ }
+ else
+ {
+ 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 = loc;
+ mo->type = MO_COPY;
+ else
+ mo->type = MO_SET;
+ mo->u.loc = CONST_CAST_RTX (expr);
+ }
+ }
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 +1915,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 +2003,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 +2024,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 +2938,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 +2961,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);