This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RTL sharing versus fwprop
>
> This is probably the best from interface side, so it would be nice if you
> can try this. Otherwise the patch looks obvious with the above
> clarifications and you may apply it until we get the new interface.
Hi,
we definitly need to make RTL infrastructure more friendly WRT sharing
rules, most of sharing bugs are so ugly to fix because they are mostly
working around infrastructure designed for RTL where sharing is allowed
everywhere.
This patch adds alternative to validate_change called
validate_unshare_change that takes care of copying the operand when
change applies. This is useful in postreload too and perhaps other
places but I didn't noticed any.
While I was on it, I updated the prototype to bools instead of ints and
also noticed that df_rescan is called unnecesarily many times for each
instructions when many changes are performed, that common while doing
CSE/GCSE.
Bootstrapped/regtested i686-linux, I am doing additional testing on the
other targets in our testers. OK?
Honza
* fwprop.c (try_fwprop_subst): Use validate_unshare_change.
* postreload.c (reload_cse_simplify_set): Instead of copying the rtx
early use validate_unshare_change.
(reload_combine): Likewise.
* recog.c (change_t): New field unshare.
(validate_change_1): Rename from validate_change; add argument unshare.
(validate_change): Turn into wrapper of validate_change_1; update
prototype for bools.
(validate_unshare_change): New.
(confirm_change_group): Unshare changes if asked for; avoid unnecesary
calls of df_insn_rescan.
* recog.h (validate_change): Replace ints by bools.
(validate_unshare_change): Declare.
Index: fwprop.c
===================================================================
*** fwprop.c (revision 125972)
--- fwprop.c (working copy)
*************** try_fwprop_subst (struct df_ref *use, rt
*** 686,692 ****
fprintf (dump_file, "\n");
}
! if (validate_change (insn, loc, new, false))
{
num_changes++;
if (dump_file)
--- 686,692 ----
fprintf (dump_file, "\n");
}
! if (validate_unshare_change (insn, loc, new, false))
{
num_changes++;
if (dump_file)
Index: postreload.c
===================================================================
*** postreload.c (revision 125972)
--- postreload.c (working copy)
*************** reload_cse_simplify_set (rtx set, rtx in
*** 345,351 ****
}
#endif
! validate_change (insn, &SET_SRC (set), copy_rtx (this_rtx), 1);
old_cost = this_cost, did_change = 1;
}
}
--- 345,351 ----
}
#endif
! validate_unshare_change (insn, &SET_SRC (set), this_rtx, 1);
old_cost = this_cost, did_change = 1;
}
}
*************** reload_combine (void)
*** 881,891 ****
with REG_SUM. */
for (i = reg_state[regno].use_index;
i < RELOAD_COMBINE_MAX_USES; i++)
! validate_change (reg_state[regno].reg_use[i].insn,
! reg_state[regno].reg_use[i].usep,
! /* Each change must have its own
! replacement. */
! copy_rtx (reg_sum), 1);
if (apply_change_group ())
{
--- 881,891 ----
with REG_SUM. */
for (i = reg_state[regno].use_index;
i < RELOAD_COMBINE_MAX_USES; i++)
! validate_unshare_change (reg_state[regno].reg_use[i].insn,
! reg_state[regno].reg_use[i].usep,
! /* Each change must have its own
! replacement. */
! reg_sum, 1);
if (apply_change_group ())
{
Index: recog.c
===================================================================
*** recog.c (revision 125972)
--- recog.c (working copy)
*************** typedef struct change_t
*** 166,171 ****
--- 166,172 ----
int old_code;
rtx *loc;
rtx old;
+ bool unshare;
} change_t;
static change_t *changes;
*************** static int num_changes = 0;
*** 191,198 ****
is not valid for the machine, suppress the change and return zero.
Otherwise, perform the change and return 1. */
! int
! validate_change (rtx object, rtx *loc, rtx new, int in_group)
{
rtx old = *loc;
--- 192,199 ----
is not valid for the machine, suppress the change and return zero.
Otherwise, perform the change and return 1. */
! static bool
! validate_change_1 (rtx object, rtx *loc, rtx new, bool in_group, bool unshare)
{
rtx old = *loc;
*************** validate_change (rtx object, rtx *loc, r
*** 219,224 ****
--- 220,226 ----
changes[num_changes].object = object;
changes[num_changes].loc = loc;
changes[num_changes].old = old;
+ changes[num_changes].unshare = unshare;
if (object && !MEM_P (object))
{
*************** validate_change (rtx object, rtx *loc, r
*** 239,244 ****
--- 241,265 ----
return apply_change_group ();
}
+ /* Wrapper for validate_change_1 without the UNSHARE argument defaulting
+ UNSHARE to false. */
+
+ bool
+ validate_change (rtx object, rtx *loc, rtx new, bool in_group)
+ {
+ return validate_change_1 (object, loc, new, in_group, false);
+ }
+
+ /* Wrapper for validate_change_1 without the UNSHARE argument defaulting
+ UNSHARE to true. */
+
+ bool
+ validate_unshare_change (rtx object, rtx *loc, rtx new, bool in_group)
+ {
+ return validate_change_1 (object, loc, new, in_group, true);
+ }
+
+
/* Keep X canonicalized if some changes have made it non-canonical; only
modifies the operands of X, not (for example) its code. Simplifications
are not the job of this routine.
*************** void
*** 414,427 ****
confirm_change_group (void)
{
int i;
for (i = 0; i < num_changes; i++)
{
rtx object = changes[i].object;
! if (object && INSN_P (object))
! df_insn_rescan (object);
}
num_changes = 0;
}
--- 435,461 ----
confirm_change_group (void)
{
int i;
+ rtx last_object = NULL;
for (i = 0; i < num_changes; i++)
{
rtx object = changes[i].object;
!
! if (changes[i].unshare)
! *changes[i].loc = copy_rtx (*changes[i].loc);
!
! /* Avoid unnecesary rescaning when multiple changes to same instruction
! are made. */
! if (object)
! {
! if (object != last_object && last_object && INSN_P (last_object))
! df_insn_rescan (last_object);
! last_object = object;
! }
}
+ if (last_object && INSN_P (last_object))
+ df_insn_rescan (last_object);
num_changes = 0;
}
Index: recog.h
===================================================================
*** recog.h (revision 125972)
--- recog.h (working copy)
*************** extern void init_recog (void);
*** 73,79 ****
extern void init_recog_no_volatile (void);
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *);
! extern int validate_change (rtx, rtx *, rtx, int);
extern bool canonicalize_change_group (rtx insn, rtx x);
extern int insn_invalid_p (rtx);
extern int verify_changes (int);
--- 73,80 ----
extern void init_recog_no_volatile (void);
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *);
! extern bool validate_change (rtx, rtx *, rtx, bool);
! extern bool validate_unshare_change (rtx, rtx *, rtx, bool);
extern bool canonicalize_change_group (rtx insn, rtx x);
extern int insn_invalid_p (rtx);
extern int verify_changes (int);