| Message ID | 201012061925.26662.ebotcazou@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 6 Dec 2010, Eric Botcazou wrote: > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -2496,6 +2496,8 @@ looking for one that is valid, and will reload one or > both registers > only if neither labeling works. > @end defmac > > +@hook TARGET_PREFERRED_RENAME_CLASS > + > @hook TARGET_PREFERRED_RELOAD_CLASS > A target hook that places additional restrictions on the register class > to use when it is necessary to copy value @var{x} into a register in class > > It looks like most of the hooks have some text in tm.texi.in so I'd suggest > writing a single sentence in target.def and move the rest of the text (with > the example) to tm.texi.in. No! Never do that for a new hook. That should only ever be done if the hook documentation is based on pre-existing GFDL-only text, while we are waiting for the FSF Board of Directors to resolve the problems with not being able to move text from one place to another. target.def is always the preferred place for hook documentation where legally possible.
> No! Never do that for a new hook. That should only ever be done if the > hook documentation is based on pre-existing GFDL-only text, while we are > waiting for the FSF Board of Directors to resolve the problems with not > being able to move text from one place to another. target.def is always > the preferred place for hook documentation where legally possible. OK, thanks for the heads up. Is that written down somewhere?
On Mon, 6 Dec 2010, Eric Botcazou wrote: > > No! Never do that for a new hook. That should only ever be done if the > > hook documentation is based on pre-existing GFDL-only text, while we are > > waiting for the FSF Board of Directors to resolve the problems with not > > being able to move text from one place to another. target.def is always > > the preferred place for hook documentation where legally possible. > > OK, thanks for the heads up. Is that written down somewhere? There's a long comment near the top of target.def discussing where documentation should go in different cases.
> There's a long comment near the top of target.def discussing where > documentation should go in different cases. Thanks. "near" is the important word here, I had looked at the top of this file, but the long comment doesn't even start in the first page!
On 12/07/2010 02:25 AM, Eric Botcazou wrote: >> Seems I didn't understand targethook fully before. You are right. Done. > > Thanks. The patch is OK for mainline modulo the following last nits: > > No need to re-submit: make the above changes, check that you still get the > desired effect with the hook for the ARM, and bootstrap/regtest on a native > platform. Thanks for your patience. > Bootstrap and regression tested on x86_64-linux. Committed. Thanks a lot for your patient review.
--- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -2496,6 +2496,8 @@ looking for one that is valid, and will reload one or both registers only if neither labeling works. @end defmac +@hook TARGET_PREFERRED_RENAME_CLASS + @hook TARGET_PREFERRED_RELOAD_CLASS A target hook that places additional restrictions on the register class to use when it is necessary to copy value @var{x} into a register in class It looks like most of the hooks have some text in tm.texi.in so I'd suggest writing a single sentence in target.def and move the rest of the text (with the example) to tm.texi.in. diff --git a/gcc/regrename.c b/gcc/regrename.c index 2535ab7..adbcde5 100644 --- a/gcc/regrename.c +++ b/gcc/regrename.c @@ -38,6 +38,7 @@ #include "timevar.h" #include "tree-pass.h" #include "df.h" +#include "target.h" You also need to add $(TARGET_H) to the regrename.o rule in Makefile.in.
> Seems I didn't understand targethook fully before. You are right. Done. Thanks. The patch is OK for mainline modulo the following last nits: + for (i = nregs - 1; i >= 0; --i) + if (TEST_HARD_REG_BIT (this_unavailable, new_reg + i) + || fixed_regs[new_reg + i] + || global_regs[new_reg + i] + /* Can't use regs which aren't saved by the prologue. */ + || (! df_regs_ever_live_p (new_reg + i) + && ! call_used_regs[new_reg + i]) +#ifdef LEAF_REGISTERS + /* We can't use a non-leaf register if we're in a + leaf function. */ + || (current_function_is_leaf + && !LEAF_REGISTERS[new_reg + i]) +#endif +#ifdef HARD_REGNO_RENAME_OK + || ! HARD_REGNO_RENAME_OK (reg + i, new_reg + i) +#endif + ) + break; + if (i >= 0) + return false; Just use "return false" instead of "break" and remove the "if (i >= 0)" test. + /* See whether it accepts all modes that occur in + definition and uses. */ + for (tmp = this_head->first; tmp; tmp = tmp->next_use) + if ((! HARD_REGNO_MODE_OK (new_reg, GET_MODE (*tmp->loc)) + && ! DEBUG_INSN_P (tmp->insn)) + || (this_head->need_caller_save_reg + && ! (HARD_REGNO_CALL_PART_CLOBBERED + (reg, GET_MODE (*tmp->loc))) + && (HARD_REGNO_CALL_PART_CLOBBERED + (new_reg, GET_MODE (*tmp->loc))))) + break; Likewise, you want "return false" here instead of "break". + /* The register iteration order here is "preferred-register-first". + Firstly(pass == 0), we iterate registers belong to PREFERRED_CLASS, + if we find a new register, we stop immeidately. immediately + /* Iterate registers first in prefered class. */ /* First iterate over registers in our preferred class. */ +reg_class_t +default_preferred_rename_class (reg_class_t rclass) +{ + return NO_REGS; +} + You'll probably need ATTRIBUTE_UNUSED to be able to bootstrap that. Also add the standard comment: /* The default implementation of TARGET_PREFERRED_RENAME_CLASS. */ in front of it. No need to re-submit: make the above changes, check that you still get the desired effect with the hook for the ARM, and bootstrap/regtest on a native platform. Thanks for your patience.