Patchwork [Ping,0/3] New macro PREFERRED_RENAME_CLASS

login
register
mail settings
Submitter Eric Botcazou
Date Dec. 6, 2010, 6:25 p.m.
Message ID <201012061925.26662.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/74421/
State New
Headers show

Comments

Eric Botcazou - Dec. 6, 2010, 6:25 p.m.
> 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.
Joseph S. Myers - Dec. 6, 2010, 8:30 p.m.
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.
Eric Botcazou - Dec. 6, 2010, 8:32 p.m.
> 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?
Joseph S. Myers - Dec. 6, 2010, 8:37 p.m.
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.
Eric Botcazou - Dec. 6, 2010, 9:59 p.m.
> 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!
Yao Qi - Dec. 7, 2010, 12:30 p.m.
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.

Patch

--- 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.