On 06/18/2015 11:32 AM, Eric Botcazou wrote:
The attached patch teaches regrename to validate insns affected by each
register renaming before making the change. I can see at least two
other ways to handle this -- earlier, by rejecting renamings that result
in invalid instructions when it's searching for the best renaming; or
later, by validating the entire set of renamings as a group instead of
incrementally for each one -- but doing it all in regname_do_replace
seems least disruptive and risky in terms of the existing code.
OK, but the patch looks incomplete, rename_chains should be adjusted as well,
i.e. regrename_do_replace should now return a boolean.
Like this? I tested this on nios2 and x86_64-linux-gnu, as before, plus
built for aarch64-linux-gnu and ran the gcc testsuite.
The c6x back end also calls regrename_do_replace. I am not set up to
build or test on that target, and Bernd told me off-list that it would
never fail on that target anyway so I have left that code alone.
-Sandra
2015-06-23 Chung-Lin Tang <clt...@codesourcery.com>
Sandra Loosemore <san...@codesourcery.com>
gcc/
* regrename.h (regrename_do_replace): Change to return bool.
* regrename.c (rename_chains): Check return value of
regname_do_replace.
(regrename_do_replace): Re-validate the modified insns and
return bool status.
* config/aarch64/cortex-a57-fma-steering.c (rename_single_chain):
Update to match rename_chains changes.
Index: gcc/regrename.h
===================================================================
--- gcc/regrename.h (revision 224700)
+++ gcc/regrename.h (working copy)
@@ -91,6 +91,6 @@ extern void regrename_analyze (bitmap);
extern du_head_p regrename_chain_from_id (unsigned int);
extern int find_rename_reg (du_head_p, enum reg_class, HARD_REG_SET *, int,
bool);
-extern void regrename_do_replace (du_head_p, int);
+extern bool regrename_do_replace (du_head_p, int);
#endif
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c (revision 224700)
+++ gcc/regrename.c (working copy)
@@ -496,12 +496,20 @@ rename_chains (void)
continue;
}
- if (dump_file)
- fprintf (dump_file, ", renamed as %s\n", reg_names[best_new_reg]);
-
- regrename_do_replace (this_head, best_new_reg);
- tick[best_new_reg] = ++this_tick;
- df_set_regs_ever_live (best_new_reg, true);
+ if (regrename_do_replace (this_head, best_new_reg))
+ {
+ if (dump_file)
+ fprintf (dump_file, ", renamed as %s\n", reg_names[best_new_reg]);
+ tick[best_new_reg] = ++this_tick;
+ df_set_regs_ever_live (best_new_reg, true);
+ }
+ else
+ {
+ if (dump_file)
+ fprintf (dump_file, ", renaming as %s failed\n",
+ reg_names[best_new_reg]);
+ tick[reg] = ++this_tick;
+ }
}
}
@@ -927,7 +935,13 @@ regrename_analyze (bitmap bb_mask)
bb->aux = NULL;
}
-void
+/* Attempt to replace all uses of the register in the chain beginning with
+ HEAD with REG. Returns true on success and false if the replacement is
+ rejected because the insns would not validate. The latter can happen
+ e.g. if a match_parallel predicate enforces restrictions on register
+ numbering in its subpatterns. */
+
+bool
regrename_do_replace (struct du_head *head, int reg)
{
struct du_chain *chain;
@@ -941,22 +955,26 @@ regrename_do_replace (struct du_head *he
int reg_ptr = REG_POINTER (*chain->loc);
if (DEBUG_INSN_P (chain->insn) && REGNO (*chain->loc) != base_regno)
- INSN_VAR_LOCATION_LOC (chain->insn) = gen_rtx_UNKNOWN_VAR_LOC ();
+ validate_change (chain->insn, &(INSN_VAR_LOCATION_LOC (chain->insn)),
+ gen_rtx_UNKNOWN_VAR_LOC (), true);
else
{
- *chain->loc = gen_raw_REG (GET_MODE (*chain->loc), reg);
+ validate_change (chain->insn, chain->loc,
+ gen_raw_REG (GET_MODE (*chain->loc), reg), true);
if (regno >= FIRST_PSEUDO_REGISTER)
ORIGINAL_REGNO (*chain->loc) = regno;
REG_ATTRS (*chain->loc) = attr;
REG_POINTER (*chain->loc) = reg_ptr;
}
-
- df_insn_rescan (chain->insn);
}
+ if (!apply_change_group ())
+ return false;
+
mode = GET_MODE (*head->first->loc);
head->regno = reg;
head->nregs = hard_regno_nregs[reg][mode];
+ return true;
}
Index: gcc/config/aarch64/cortex-a57-fma-steering.c
===================================================================
--- gcc/config/aarch64/cortex-a57-fma-steering.c (revision 224700)
+++ gcc/config/aarch64/cortex-a57-fma-steering.c (working copy)
@@ -288,11 +288,19 @@ rename_single_chain (du_head_p head, HAR
return false;
}
- if (dump_file)
- fprintf (dump_file, ", renamed as %s\n", reg_names[best_new_reg]);
-
- regrename_do_replace (head, best_new_reg);
- df_set_regs_ever_live (best_new_reg, true);
+ if (regrename_do_replace (head, best_new_reg))
+ {
+ if (dump_file)
+ fprintf (dump_file, ", renamed as %s\n", reg_names[best_new_reg]);
+ df_set_regs_ever_live (best_new_reg, true);
+ }
+ else
+ {
+ if (dump_file)
+ fprintf (dump_file, ", renaming as %s failed\n",
+ reg_names[best_new_reg]);
+ return false;
+ }
return true;
}
