This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [4.5] Better support for widening multiplies
> One small problem in using forward_propagate_into is that the
> infrastructure (especially try_fwprop_subst) is designed for propagating
> from a single insn. In your case you need to propagate into all the
> uses of the multiplication insn. I'm bootstrapping a refactoring patch
> that splits try_fwprop_subst so that you can use its parts (making
> changes, validating them, updating dataflow) separately.
Here it is. If you approve and use this patch, let me know if you'd
like me to commit it or you'll do it yourself. If you do not use it, I
may actually not commit it at all since the code we have now works.
Bootstrapped/regtested i686-pc-linux-gnu (there are a few testcases,
such as the one for PR35281, that make sure fwprop is not totally borked!).
Paolo
2008-02-19 Paolo Bonzini <bonzini@gnu.org>
* fwprop.c (update_df_1): New name of update_df. Do not call
df_insn_rescan, return whether changes were made.
(set_insn_cost): New.
(finish_fwprop, update_df): New, extracted from try_fwprop_subst.
(try_fwprop_subst): Use them.
(find_fwprop_def): New, from forward_propagate_into.
(forward_propagate_into): Use it.
Index: fwprop.c
===================================================================
--- fwprop.c (revision 144226)
+++ fwprop.c (working copy)
@@ -681,9 +681,9 @@ find_occurrence (rtx *px, rtx find)
uses from USE_VEC. Find those that are present, and create new items
in the data flow object of the pass. Mark any new uses as having the
given TYPE. */
-static void
-update_df (rtx insn, rtx *loc, df_ref *use_rec, enum df_ref_type type,
- int new_flags)
+static bool
+update_df_1 (rtx insn, rtx *loc, df_ref *use_rec, enum df_ref_type type,
+ int new_flags)
{
bool changed = false;
@@ -719,38 +719,48 @@ update_df (rtx insn, rtx *loc, df_ref *u
df_chain_copy (new_use, DF_REF_CHAIN (orig_use));
changed = true;
}
- if (changed)
- df_insn_rescan (insn);
+
+ return changed;
}
+enum fwprop_result {
+ /* The propagation failed to be recognized or was unprofitable. */
+ FWP_FAILURE = 0,
+
+ /* The propagation succeeded. */
+ FWP_SUCCESS = 1,
+
+ /* The propagation failed but was used to add a REG_EQUAL note. */
+ FWP_NOTE = 2
+};
-/* Try substituting NEW into LOC, which originated from forward propagation
- of USE's value from DEF_INSN. SET_REG_EQUAL says whether we are
- substituting the whole SET_SRC, so we can set a REG_EQUAL note if the
- new insn is not recognized. Return whether the substitution was
- performed. */
-static bool
-try_fwprop_subst (df_ref use, rtx *loc, rtx new_rtx, rtx def_insn, bool set_reg_equal)
+/* Return the cost of the single set within INSN. */
+
+static inline int
+set_insn_cost (rtx insn)
{
- rtx insn = DF_REF_INSN (use);
- enum df_ref_type type = DF_REF_TYPE (use);
- int flags = DF_REF_FLAGS (use);
rtx set = single_set (insn);
bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn));
- int old_cost = rtx_cost (SET_SRC (set), SET, speed);
- bool ok;
+ return rtx_cost (SET_SRC (set), SET, speed);
+}
- if (dump_file)
- {
- fprintf (dump_file, "\nIn insn %d, replacing\n ", INSN_UID (insn));
- print_inline_rtx (dump_file, *loc, 2);
- fprintf (dump_file, "\n with ");
- print_inline_rtx (dump_file, new_rtx, 2);
- fprintf (dump_file, "\n");
- }
- validate_unshare_change (insn, loc, new_rtx, true);
+/* Finish a forward propagation into INSN.
+
+ OLD_COST is -1 if the cost of the insn should be disregarded,
+ otherwise OLD_COST is compared with the cost of the replacement and
+ the substitution fails if the change is not profitable.
+
+ SET_REG_EQUAL says whether we are substituting the whole SET_SRC, so
+ we can se a REG_EQUAL note if the new insn is not recognized. Return a
+ fwprop_result saying whether the substitution was performed. */
+
+static int
+finish_fwprop (rtx insn, int old_cost, bool set_reg_equal)
+{
+ bool ok;
+
if (!verify_changes (0))
{
if (dump_file)
@@ -759,8 +764,7 @@ try_fwprop_subst (df_ref use, rtx *loc,
ok = false;
}
- else if (DF_REF_TYPE (use) == DF_REF_REG_USE
- && rtx_cost (SET_SRC (set), SET, speed) > old_cost)
+ else if (old_cost != -1 && set_insn_cost (insn) > old_cost)
{
if (dump_file)
fprintf (dump_file, "Changes to insn %d not profitable\n",
@@ -779,14 +783,7 @@ try_fwprop_subst (df_ref use, rtx *loc,
{
confirm_change_group ();
num_changes++;
-
- df_ref_remove (use);
- if (!CONSTANT_P (new_rtx))
- {
- struct df_insn_info *insn_info = DF_INSN_INFO_GET (def_insn);
- update_df (insn, loc, DF_INSN_INFO_USES (insn_info), type, flags);
- update_df (insn, loc, DF_INSN_INFO_EQ_USES (insn_info), type, flags);
- }
+ return FWP_SUCCESS;
}
else
{
@@ -796,28 +793,88 @@ try_fwprop_subst (df_ref use, rtx *loc,
making a new one if one does not already exist. */
if (set_reg_equal)
{
+ rtx set, new_rtx;
if (dump_file)
fprintf (dump_file, " Setting REG_EQUAL note\n");
+ set = single_set (insn);
+ new_rtx = SET_SRC (set);
set_unique_reg_note (insn, REG_EQUAL, copy_rtx (new_rtx));
-
- /* ??? Is this still necessary if we add the note through
- set_unique_reg_note? */
- if (!CONSTANT_P (new_rtx))
- {
- struct df_insn_info *insn_info = DF_INSN_INFO_GET (def_insn);
- update_df (insn, loc, DF_INSN_INFO_USES (insn_info),
- type, DF_REF_IN_NOTE);
- update_df (insn, loc, DF_INSN_INFO_EQ_USES (insn_info),
- type, DF_REF_IN_NOTE);
- }
+ return FWP_NOTE;
}
+ else
+ return FWP_FAILURE;
}
+}
+
- return ok;
+/* Update the dataflow info after replacing *LOC (which contained USE)
+ with the source of DEF_INSN. RESULT is what finish_fwprop returned. */
+
+static void
+update_df (df_ref use, rtx *loc, rtx def_insn, enum fwprop_result result)
+{
+ rtx insn = DF_REF_INSN (use);
+ struct df_insn_info *insn_info = DF_INSN_INFO_GET (def_insn);
+ enum df_ref_type type = DF_REF_TYPE (use);
+ enum df_ref_flags flags;
+ bool changed;
+
+ if (result == FWP_NOTE)
+ flags = DF_REF_IN_NOTE;
+ else
+ flags = DF_REF_FLAGS (use);
+
+ changed = update_df_1 (insn, loc, DF_INSN_INFO_USES (insn_info),
+ type, flags);
+ changed |= update_df_1 (insn, loc, DF_INSN_INFO_EQ_USES (insn_info),
+ type, flags);
+
+ if (changed)
+ df_insn_rescan (insn);
}
+/* Try substituting NEW into LOC, which originated from forward propagation
+ of USE's value from DEF_INSN. SET_REG_EQUAL says whether we are
+ substituting the whole SET_SRC, so we can set a REG_EQUAL note if the
+ new insn is not recognized. Return whether the substitution was
+ performed. */
+
+static bool
+try_fwprop_subst (df_ref use, rtx *loc, rtx new_rtx, rtx def_insn, bool set_reg_equal)
+{
+ rtx insn = DF_REF_INSN (use);
+ enum fwprop_result result;
+ int old_cost;
+
+ if (dump_file)
+ {
+ fprintf (dump_file, "\nIn insn %d, replacing\n ", INSN_UID (insn));
+ print_inline_rtx (dump_file, *loc, 2);
+ fprintf (dump_file, "\n with ");
+ print_inline_rtx (dump_file, new_rtx, 2);
+ fprintf (dump_file, "\n");
+ }
+
+ if (DF_REF_TYPE (use) == DF_REF_REG_USE)
+ old_cost = set_insn_cost (insn);
+ else
+ old_cost = -1;
+
+ validate_unshare_change (insn, loc, new_rtx, true);
+ result = finish_fwprop (insn, old_cost, set_reg_equal);
+
+ if (result == FWP_FAILURE)
+ return false;
+
+ if (!CONSTANT_P (new_rtx))
+ update_df (use, loc, def_insn, result);
+ if (result != FWP_NOTE)
+ df_ref_remove (use);
+ return true;
+}
+
/* If USE is a paradoxical subreg, see if it can be replaced by a pseudo. */
static bool
@@ -948,36 +1005,34 @@ forward_propagate_and_simplify (df_ref u
}
-/* Given a use USE of an insn, if it has a single reaching
- definition, try to forward propagate it into that insn. */
-
-static void
-forward_propagate_into (df_ref use)
+/* Find the def to be propagated into USE and, if the propagation can be
+ made, return it. */
+static df_ref
+find_fwprop_def (df_ref use)
{
- struct df_link *defs;
df_ref def;
- rtx def_insn, def_set, use_insn;
- rtx parent;
+ struct df_link *defs;
+ rtx parent, use_insn;
if (DF_REF_FLAGS (use) & DF_REF_READ_WRITE)
- return;
+ return NULL;
if (DF_REF_IS_ARTIFICIAL (use))
- return;
+ return NULL;
/* Only consider uses that have a single definition. */
defs = DF_REF_CHAIN (use);
if (!defs || defs->next)
- return;
+ return NULL;
def = defs->ref;
if (DF_REF_FLAGS (def) & DF_REF_READ_WRITE)
- return;
+ return NULL;
if (DF_REF_IS_ARTIFICIAL (def))
- return;
+ return NULL;
/* Do not propagate loop invariant definitions inside the loop. */
if (DF_REF_BB (def)->loop_father != DF_REF_BB (use)->loop_father)
- return;
+ return NULL;
/* Check if the use is still present in the insn! */
use_insn = DF_REF_INSN (use);
@@ -987,6 +1042,23 @@ forward_propagate_into (df_ref use)
parent = PATTERN (use_insn);
if (!reg_mentioned_p (DF_REF_REG (use), parent))
+ return NULL;
+
+ return def;
+}
+
+
+/* Given a use USE of an insn, if it has a single reaching
+ definition, try to forward propagate it into that insn. */
+
+static void
+forward_propagate_into (df_ref use)
+{
+ df_ref def;
+ rtx def_insn, def_set;
+
+ def = find_fwprop_def (use);
+ if (!def)
return;
def_insn = DF_REF_INSN (def);