public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] avoiding too narrow register classes in reload
@ 2004-12-13 21:20 Alexandre Oliva
  2004-12-14  2:06 ` Richard Henderson
  2004-12-14 16:22 ` Bernd Schmidt
  0 siblings, 2 replies; 10+ messages in thread
From: Alexandre Oliva @ 2004-12-13 21:20 UTC (permalink / raw)
  To: gcc

I see that reload avoids committing a pseudo that needs reloading to a
single-register class.  On this port I'm working on, I happen to have
one class that contains only two registers, which unfortunately happen
to be the first two registers used for argument passing and for
returning values.

So it's quite common for the register allocator to decide it's a good
idea to choose this class (or the two single-register classes for each
of the registers) as the preferred class for pseudos that are ever
passed as one of the first two arguments to a function, or that are
ever assigned the return value of a function call.

The following patchlet to reload fixes the problem I ran into.  What
the surrounding code does is to verify whether the current alternative
is a superset of the preferred class.  If so, it narrows the
alternative to this preferred class, instead of using the wider class.
This is what causes the problem I'm running into: if this alternative
is selected (and it will often be), we end up forced to use one of the
first two argument registers for the reload, and this will fail if the
reload is needed in the middle of a function call sequence.  Defining
SMALL_REGISTER_CLASSES to 1 had no effect whatsoever (at least as far
as this particular reload as concerned).

Since CLASS_LIKELY_SPILLED_P is defined, by default, to non-zero if
the register class size is 1, I figured I might as well replace tests
for the register class size such that they used
CLASS_LIKELY_SPILLED_P, without significant semantics changes.  I
couldn't decide whether to create an alternate macro or target hook to
convey the meaning that reload shouldn't choose this narrower class,
so I thought I'd run the idea through you first.

Does this look like a reasonable approach?  Should I introduce a
separate macro, or do you agree using CLASS_LIKELY_SPILLED_P
reasonable for this purpose?  Should other locations that compare
reg_class_size with 1 be adjusted similarly?

Thanks in advance for any feedback,

--- reload.c
+++ reload.c
@@ -3461,7 +3461,11 @@ find_reloads (rtx insn, int replace, int
 	  if (! win && ! did_match
 	      && this_alternative[i] != (int) NO_REGS
 	      && GET_MODE_SIZE (operand_mode[i]) <= UNITS_PER_WORD
-	      && reg_class_size[(int) preferred_class[i]] > 1)
+	      /* However, if the subset is a small class very
+		 likely to be spilled, go with the wider class
+		 anyway.  */
+	      && preferred_class[i] != NO_REGS
+	      && ! CLASS_LIKELY_SPILLED_P (preferred_class[i]))
 	    {
 	      if (! reg_class_subset_p (this_alternative[i],
 					preferred_class[i]))


-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-13 21:20 [RFC] avoiding too narrow register classes in reload Alexandre Oliva
@ 2004-12-14  2:06 ` Richard Henderson
  2004-12-14 21:20   ` Alexandre Oliva
  2004-12-14 16:22 ` Bernd Schmidt
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2004-12-14  2:06 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc

On Mon, Dec 13, 2004 at 07:20:21PM -0200, Alexandre Oliva wrote:
> Does this look like a reasonable approach?

Yes.

> Should other locations that compare
> reg_class_size with 1 be adjusted similarly?

Probably would be good.


r~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-13 21:20 [RFC] avoiding too narrow register classes in reload Alexandre Oliva
  2004-12-14  2:06 ` Richard Henderson
@ 2004-12-14 16:22 ` Bernd Schmidt
  2004-12-14 20:53   ` Alexandre Oliva
  1 sibling, 1 reply; 10+ messages in thread
From: Bernd Schmidt @ 2004-12-14 16:22 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc

> This is what causes the problem I'm running into: if this alternative
> is selected (and it will often be), we end up forced to use one of the
> first two argument registers for the reload, and this will fail if the
> reload is needed in the middle of a function call sequence.

What does the insn that it's trying to reload look like?


Bernd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-14 16:22 ` Bernd Schmidt
@ 2004-12-14 20:53   ` Alexandre Oliva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Oliva @ 2004-12-14 20:53 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: gcc

On Dec 14, 2004, bernds_cb1@t-online.de (Bernd Schmidt) wrote:

>> This is what causes the problem I'm running into: if this alternative
>> is selected (and it will often be), we end up forced to use one of the
>> first two argument registers for the reload, and this will fail if the
>> reload is needed in the middle of a function call sequence.

> What does the insn that it's trying to reload look like?

  (set (mem addr) (pseudo))

addr is a legitimate address, that doesn't require any reloads.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-14  2:06 ` Richard Henderson
@ 2004-12-14 21:20   ` Alexandre Oliva
  2004-12-14 22:31     ` Richard Henderson
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Oliva @ 2004-12-14 21:20 UTC (permalink / raw)
  To: Richard Henderson, Bernd Schmidt; +Cc: gcc, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

On Dec 14, 2004, Richard Henderson <rth@redhat.com> wrote:

> On Mon, Dec 13, 2004 at 07:20:21PM -0200, Alexandre Oliva wrote:
>> Does this look like a reasonable approach?

> Yes.

>> Should other locations that compare
>> reg_class_size with 1 be adjusted similarly?

> Probably would be good.

Here's the patch I've bootstrapped and tested on x86_64-linux-gnu.
There's one use of reg_class_size that I wasn't sure whether to
replace with the new macro, in combine_reloads():

							     rld[output_reload].out))))
	&& ! reload_inner_reg_of_subreg (rld[i].in, rld[i].inmode,
					 rld[i].when_needed != RELOAD_FOR_INPUT)
	&& (reg_class_size[(int) rld[i].class] || SMALL_REGISTER_CLASSES)
	/* We will allow making things slightly worse by combining an
	   input and an output, but no worse than that.  */
	&& (rld[i].when_needed == RELOAD_FOR_INPUT

The logic was different from all other cases, that tested for
reg_class_size[...] == 1 || SMALL_REGISTER_CLASSES, and I couldn't
tell whether it was on purpose or a typo.  Can anyone?

I realize the patch introduces some possibly-significant semantic
changes, in that NO_REGS is now regarded as a SMALL_REGISTER_CLASS_P
and so it passes a number of cases where we tested for
reg_class_size[...] == 1 || SMALL_REGISTER_CLASSES.  Since
SMALL_REGISTER_CLASSES would pass, I figured this couldn't be a
problem.  The last hunk, where || SMALL_REGISTER_CLASSES wasn't
present, uses the new predicate but also ensures that reg_class_size
is non-zero (it broke otherwise).  Does it make sense to narrow the
predicate to hold only for non-empty classes, and adjust the
one-before-last hunk, that uses the negated predicate in a way that
requires it to hold for empty classes?

Ok to install this for now?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reload-small-regclass-likely-spilled.patch --]
[-- Type: text/x-patch, Size: 4133 bytes --]

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* reload.c (SMALL_REGISTER_CLASS_P): New.
	(push_secondary_reload, find_reusable_reload, find_reloads): Use
	it instead of testing only the class size.

Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.260
diff -u -p -r1.260 reload.c
--- gcc/reload.c 7 Dec 2004 01:14:40 -0000 1.260
+++ gcc/reload.c 14 Dec 2004 21:16:57 -0000
@@ -113,6 +113,12 @@ a register with any other reload.  */
   (CONSTANT_P (X)				\
    && GET_CODE (X) != HIGH			\
    && !targetm.cannot_force_const_mem (X))
+
+/* True if C is a register class that has too few registers to be
+   safely used as a reload target class.  */
+#define SMALL_REGISTER_CLASS_P(C) \
+  (reg_class_size [(int)(C)] <= 1 || CLASS_LIKELY_SPILLED_P (C))
+
 \f
 /* All reloads of the current insn are recorded here.  See reload.h for
    comments.  */
@@ -443,7 +449,7 @@ push_secondary_reload (int in_p, rtx x, 
 			  == CODE_FOR_nothing))
 		|| (! in_p &&(rld[t_reload].secondary_out_icode
 			      == CODE_FOR_nothing)))
-	    && (reg_class_size[(int) t_class] == 1 || SMALL_REGISTER_CLASSES)
+	    && (SMALL_REGISTER_CLASS_P (t_class) || SMALL_REGISTER_CLASSES)
 	    && MERGABLE_RELOADS (secondary_type,
 				 rld[t_reload].when_needed,
 				 opnum, rld[t_reload].opnum))
@@ -501,7 +507,7 @@ push_secondary_reload (int in_p, rtx x, 
 	    || (! in_p && rld[s_reload].secondary_out_reload == t_reload))
 	&& ((in_p && rld[s_reload].secondary_in_icode == t_icode)
 	    || (! in_p && rld[s_reload].secondary_out_icode == t_icode))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (secondary_type, rld[s_reload].when_needed,
 			     opnum, rld[s_reload].opnum))
       {
@@ -755,7 +761,7 @@ find_reusable_reload (rtx *p_in, rtx out
 	    || (out != 0 && MATCHES (rld[i].out, out)
 		&& (in == 0 || rld[i].in == 0 || MATCHES (rld[i].in, in))))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed, opnum, rld[i].opnum))
       return i;
 
@@ -780,7 +786,7 @@ find_reusable_reload (rtx *p_in, rtx out
 		&& GET_RTX_CLASS (GET_CODE (in)) == RTX_AUTOINC
 		&& MATCHES (XEXP (in, 0), rld[i].in)))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed,
 			     opnum, rld[i].opnum))
       {
@@ -1768,8 +1774,7 @@ combine_reloads (void)
 							     rld[output_reload].out))))
 	&& ! reload_inner_reg_of_subreg (rld[i].in, rld[i].inmode,
 					 rld[i].when_needed != RELOAD_FOR_INPUT)
-	&& (reg_class_size[(int) rld[i].class]
-	    || SMALL_REGISTER_CLASSES)
+	&& (reg_class_size[(int) rld[i].class] || SMALL_REGISTER_CLASSES)
 	/* We will allow making things slightly worse by combining an
 	   input and an output, but no worse than that.  */
 	&& (rld[i].when_needed == RELOAD_FOR_INPUT
@@ -3484,7 +3489,7 @@ find_reloads (rtx insn, int replace, int
 	  if (! win && ! did_match
 	      && this_alternative[i] != (int) NO_REGS
 	      && GET_MODE_SIZE (operand_mode[i]) <= UNITS_PER_WORD
-	      && reg_class_size[(int) preferred_class[i]] > 1)
+	      && ! SMALL_REGISTER_CLASS_P (preferred_class[i]))
 	    {
 	      if (! reg_class_subset_p (this_alternative[i],
 					preferred_class[i]))
@@ -3542,7 +3547,8 @@ find_reloads (rtx insn, int replace, int
 		{
 		  /* If the output is in a single-reg class,
 		     it's costly to reload it, so reload the input instead.  */
-		  if (reg_class_size[this_alternative[i]] == 1
+		  if (reg_class_size[this_alternative[i]] > 0
+		      && SMALL_REGISTER_CLASS_P (this_alternative[i])
 		      && (REG_P (recog_data.operand[j])
 			  || GET_CODE (recog_data.operand[j]) == SUBREG))
 		    {

[-- Attachment #3: Type: text/plain, Size: 188 bytes --]


-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-14 21:20   ` Alexandre Oliva
@ 2004-12-14 22:31     ` Richard Henderson
  2004-12-15  6:56       ` Alexandre Oliva
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2004-12-14 22:31 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Bernd Schmidt, gcc, gcc-patches

On Tue, Dec 14, 2004 at 07:20:10PM -0200, Alexandre Oliva wrote:
> I realize the patch introduces some possibly-significant semantic
> changes, in that NO_REGS is now regarded as a SMALL_REGISTER_CLASS_P

I think that is indeed a surprising result.

> Does it make sense to narrow the
> predicate to hold only for non-empty classes, and adjust the
> one-before-last hunk, that uses the negated predicate in a way that
> requires it to hold for empty classes?

Please.


r~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-14 22:31     ` Richard Henderson
@ 2004-12-15  6:56       ` Alexandre Oliva
  2004-12-15  8:09         ` Richard Henderson
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Oliva @ 2004-12-15  6:56 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Bernd Schmidt, gcc, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

On Dec 14, 2004, Richard Henderson <rth@redhat.com> wrote:

> On Tue, Dec 14, 2004 at 07:20:10PM -0200, Alexandre Oliva wrote:
>> I realize the patch introduces some possibly-significant semantic
>> changes, in that NO_REGS is now regarded as a SMALL_REGISTER_CLASS_P

> I think that is indeed a surprising result.

>> Does it make sense to narrow the
>> predicate to hold only for non-empty classes, and adjust the
>> one-before-last hunk, that uses the negated predicate in a way that
>> requires it to hold for empty classes?

> Please.

Here's the revised patch.  Bootstrapped on x86_64-linux-gnu,
regtesting will still take a while to complete.  Ok to install if no
regressions show up?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reload-small-regclass-likely-spilled-take2.patch --]
[-- Type: text/x-patch, Size: 4232 bytes --]

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* reload.c (SMALL_REGISTER_CLASS_P): New.
	(push_secondary_reload, find_reusable_reload, find_reloads): Use
	it instead of testing only the class size.

Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.260
diff -u -p -r1.260 reload.c
--- gcc/reload.c 7 Dec 2004 01:14:40 -0000 1.260
+++ gcc/reload.c 15 Dec 2004 02:20:54 -0000
@@ -113,6 +113,13 @@ a register with any other reload.  */
   (CONSTANT_P (X)				\
    && GET_CODE (X) != HIGH			\
    && !targetm.cannot_force_const_mem (X))
+
+/* True if C is a register class that has too few registers to be
+   safely used as a reload target class.  */
+#define SMALL_REGISTER_CLASS_P(C) \
+  (reg_class_size [(int)(C)] == 1 \
+   || (reg_class_size [(int)(C)] >= 1 && CLASS_LIKELY_SPILLED_P (C)))
+
 \f
 /* All reloads of the current insn are recorded here.  See reload.h for
    comments.  */
@@ -443,7 +450,7 @@ push_secondary_reload (int in_p, rtx x, 
 			  == CODE_FOR_nothing))
 		|| (! in_p &&(rld[t_reload].secondary_out_icode
 			      == CODE_FOR_nothing)))
-	    && (reg_class_size[(int) t_class] == 1 || SMALL_REGISTER_CLASSES)
+	    && (SMALL_REGISTER_CLASS_P (t_class) || SMALL_REGISTER_CLASSES)
 	    && MERGABLE_RELOADS (secondary_type,
 				 rld[t_reload].when_needed,
 				 opnum, rld[t_reload].opnum))
@@ -501,7 +508,7 @@ push_secondary_reload (int in_p, rtx x, 
 	    || (! in_p && rld[s_reload].secondary_out_reload == t_reload))
 	&& ((in_p && rld[s_reload].secondary_in_icode == t_icode)
 	    || (! in_p && rld[s_reload].secondary_out_icode == t_icode))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (secondary_type, rld[s_reload].when_needed,
 			     opnum, rld[s_reload].opnum))
       {
@@ -755,7 +762,7 @@ find_reusable_reload (rtx *p_in, rtx out
 	    || (out != 0 && MATCHES (rld[i].out, out)
 		&& (in == 0 || rld[i].in == 0 || MATCHES (rld[i].in, in))))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed, opnum, rld[i].opnum))
       return i;
 
@@ -780,7 +787,7 @@ find_reusable_reload (rtx *p_in, rtx out
 		&& GET_RTX_CLASS (GET_CODE (in)) == RTX_AUTOINC
 		&& MATCHES (XEXP (in, 0), rld[i].in)))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed,
 			     opnum, rld[i].opnum))
       {
@@ -1768,8 +1775,7 @@ combine_reloads (void)
 							     rld[output_reload].out))))
 	&& ! reload_inner_reg_of_subreg (rld[i].in, rld[i].inmode,
 					 rld[i].when_needed != RELOAD_FOR_INPUT)
-	&& (reg_class_size[(int) rld[i].class]
-	    || SMALL_REGISTER_CLASSES)
+	&& (reg_class_size[(int) rld[i].class] || SMALL_REGISTER_CLASSES)
 	/* We will allow making things slightly worse by combining an
 	   input and an output, but no worse than that.  */
 	&& (rld[i].when_needed == RELOAD_FOR_INPUT
@@ -3484,7 +3490,8 @@ find_reloads (rtx insn, int replace, int
 	  if (! win && ! did_match
 	      && this_alternative[i] != (int) NO_REGS
 	      && GET_MODE_SIZE (operand_mode[i]) <= UNITS_PER_WORD
-	      && reg_class_size[(int) preferred_class[i]] > 1)
+	      && reg_class_size [(int) preferred_class[i]] > 0
+	      && ! SMALL_REGISTER_CLASS_P (preferred_class[i]))
 	    {
 	      if (! reg_class_subset_p (this_alternative[i],
 					preferred_class[i]))
@@ -3542,7 +3549,8 @@ find_reloads (rtx insn, int replace, int
 		{
 		  /* If the output is in a single-reg class,
 		     it's costly to reload it, so reload the input instead.  */
-		  if (reg_class_size[this_alternative[i]] == 1
+		  if (reg_class_size[this_alternative[i]] > 0
+		      && SMALL_REGISTER_CLASS_P (this_alternative[i])
 		      && (REG_P (recog_data.operand[j])
 			  || GET_CODE (recog_data.operand[j]) == SUBREG))
 		    {

[-- Attachment #3: Type: text/plain, Size: 188 bytes --]


-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-15  6:56       ` Alexandre Oliva
@ 2004-12-15  8:09         ` Richard Henderson
  2004-12-15 20:07           ` Alexandre Oliva
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2004-12-15  8:09 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Bernd Schmidt, gcc, gcc-patches

On Wed, Dec 15, 2004 at 04:56:08AM -0200, Alexandre Oliva wrote:
> 	* reload.c (SMALL_REGISTER_CLASS_P): New.
> 	(push_secondary_reload, find_reusable_reload, find_reloads): Use
> 	it instead of testing only the class size.

Ok, except,

> +#define SMALL_REGISTER_CLASS_P(C) \
> +  (reg_class_size [(int)(C)] == 1 \
> +   || (reg_class_size [(int)(C)] >= 1 && CLASS_LIKELY_SPILLED_P (C)))

We no longer have to cater to broken K&R compilers that refuse to
index arrays by enums.  Kill the casts.

> -		  if (reg_class_size[this_alternative[i]] == 1
> +		  if (reg_class_size[this_alternative[i]] > 0
> +		      && SMALL_REGISTER_CLASS_P (this_alternative[i])

reg_class_size check redundant with SMALL_REGISTER_CLASS_P.


r~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] avoiding too narrow register classes in reload
  2004-12-15  8:09         ` Richard Henderson
@ 2004-12-15 20:07           ` Alexandre Oliva
  2004-12-16 14:14             ` Dave Korn
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Oliva @ 2004-12-15 20:07 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Bernd Schmidt, gcc, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

On Dec 15, 2004, Richard Henderson <rth@redhat.com> wrote:

> On Wed, Dec 15, 2004 at 04:56:08AM -0200, Alexandre Oliva wrote:
>> * reload.c (SMALL_REGISTER_CLASS_P): New.
>> (push_secondary_reload, find_reusable_reload, find_reloads): Use
>> it instead of testing only the class size.

> Ok, except,

> We no longer have to cater to broken K&R compilers that refuse to
> index arrays by enums.  Kill the casts.

Ok

>> -		  if (reg_class_size[this_alternative[i]] == 1
>> +		  if (reg_class_size[this_alternative[i]] > 0
>> +		      && SMALL_REGISTER_CLASS_P (this_alternative[i])

> reg_class_size check redundant with SMALL_REGISTER_CLASS_P.

I thought I'd leave it in for, like, documentation purposes, just in
case someone reworks SMALL_REGISTER_CLASS_P such that it can match
even for 0-sized classes, but fine.  Here's what I'm checking in.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reload-small-regclass-likely-spilled-take3.patch --]
[-- Type: text/x-patch, Size: 3821 bytes --]

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* reload.c (SMALL_REGISTER_CLASS_P): New.
	(push_secondary_reload, find_reusable_reload, find_reloads): Use
	it instead of testing only the class size.

Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.260
diff -u -p -r1.260 reload.c
--- gcc/reload.c 7 Dec 2004 01:14:40 -0000 1.260
+++ gcc/reload.c 15 Dec 2004 20:05:08 -0000
@@ -113,6 +113,13 @@ a register with any other reload.  */
   (CONSTANT_P (X)				\
    && GET_CODE (X) != HIGH			\
    && !targetm.cannot_force_const_mem (X))
+
+/* True if C is a non-empty register class that has too few registers
+   to be safely used as a reload target class.  */
+#define SMALL_REGISTER_CLASS_P(C) \
+  (reg_class_size [(C)] == 1 \
+   || (reg_class_size [(C)] >= 1 && CLASS_LIKELY_SPILLED_P (C)))
+
 \f
 /* All reloads of the current insn are recorded here.  See reload.h for
    comments.  */
@@ -443,7 +450,7 @@ push_secondary_reload (int in_p, rtx x, 
 			  == CODE_FOR_nothing))
 		|| (! in_p &&(rld[t_reload].secondary_out_icode
 			      == CODE_FOR_nothing)))
-	    && (reg_class_size[(int) t_class] == 1 || SMALL_REGISTER_CLASSES)
+	    && (SMALL_REGISTER_CLASS_P (t_class) || SMALL_REGISTER_CLASSES)
 	    && MERGABLE_RELOADS (secondary_type,
 				 rld[t_reload].when_needed,
 				 opnum, rld[t_reload].opnum))
@@ -501,7 +508,7 @@ push_secondary_reload (int in_p, rtx x, 
 	    || (! in_p && rld[s_reload].secondary_out_reload == t_reload))
 	&& ((in_p && rld[s_reload].secondary_in_icode == t_icode)
 	    || (! in_p && rld[s_reload].secondary_out_icode == t_icode))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (secondary_type, rld[s_reload].when_needed,
 			     opnum, rld[s_reload].opnum))
       {
@@ -755,7 +762,7 @@ find_reusable_reload (rtx *p_in, rtx out
 	    || (out != 0 && MATCHES (rld[i].out, out)
 		&& (in == 0 || rld[i].in == 0 || MATCHES (rld[i].in, in))))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed, opnum, rld[i].opnum))
       return i;
 
@@ -780,7 +787,7 @@ find_reusable_reload (rtx *p_in, rtx out
 		&& GET_RTX_CLASS (GET_CODE (in)) == RTX_AUTOINC
 		&& MATCHES (XEXP (in, 0), rld[i].in)))
 	&& (rld[i].out == 0 || ! earlyclobber_operand_p (rld[i].out))
-	&& (reg_class_size[(int) class] == 1 || SMALL_REGISTER_CLASSES)
+	&& (SMALL_REGISTER_CLASS_P (class) || SMALL_REGISTER_CLASSES)
 	&& MERGABLE_RELOADS (type, rld[i].when_needed,
 			     opnum, rld[i].opnum))
       {
@@ -3484,7 +3491,8 @@ find_reloads (rtx insn, int replace, int
 	  if (! win && ! did_match
 	      && this_alternative[i] != (int) NO_REGS
 	      && GET_MODE_SIZE (operand_mode[i]) <= UNITS_PER_WORD
-	      && reg_class_size[(int) preferred_class[i]] > 1)
+	      && reg_class_size [(int) preferred_class[i]] > 0
+	      && ! SMALL_REGISTER_CLASS_P (preferred_class[i]))
 	    {
 	      if (! reg_class_subset_p (this_alternative[i],
 					preferred_class[i]))
@@ -3540,9 +3548,9 @@ find_reloads (rtx insn, int replace, int
 		  && !immune_p (recog_data.operand[j], recog_data.operand[i],
 				early_data))
 		{
-		  /* If the output is in a single-reg class,
+		  /* If the output is in a non-empty few-regs class,
 		     it's costly to reload it, so reload the input instead.  */
-		  if (reg_class_size[this_alternative[i]] == 1
+		  if (SMALL_REGISTER_CLASS_P (this_alternative[i])
 		      && (REG_P (recog_data.operand[j])
 			  || GET_CODE (recog_data.operand[j]) == SUBREG))
 		    {

[-- Attachment #3: Type: text/plain, Size: 188 bytes --]


-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [RFC] avoiding too narrow register classes in reload
  2004-12-15 20:07           ` Alexandre Oliva
@ 2004-12-16 14:14             ` Dave Korn
  0 siblings, 0 replies; 10+ messages in thread
From: Dave Korn @ 2004-12-16 14:14 UTC (permalink / raw)
  To: 'Alexandre Oliva', 'Richard Henderson'
  Cc: 'Bernd Schmidt', gcc, gcc-patches

> -----Original Message-----
> From: gcc-owner On Behalf Of Alexandre Oliva
> Sent: 15 December 2004 20:07

> On Dec 15, 2004, Richard Henderson wrote:
> 

> > Ok, except,

> >> -		  if (reg_class_size[this_alternative[i]] == 1
> >> +		  if (reg_class_size[this_alternative[i]] > 0
> >> +		      && SMALL_REGISTER_CLASS_P (this_alternative[i])
> 
> > reg_class_size check redundant with SMALL_REGISTER_CLASS_P.
> 
> I thought I'd leave it in for, like, documentation purposes, just in
> case someone reworks SMALL_REGISTER_CLASS_P such that it can match
> even for 0-sized classes, but fine.  


  As long as it's not in a critical inner loop, I always feel that a couple of
memory accesses and a test-and-branch are a small price to pay for having code
that clearly self-documents the intent behind it.  But hey, no big deal :)


    cheers, 
      DaveK
-- 
Can't think of a witty .sigline today....

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2004-12-16 14:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-12-13 21:20 [RFC] avoiding too narrow register classes in reload Alexandre Oliva
2004-12-14  2:06 ` Richard Henderson
2004-12-14 21:20   ` Alexandre Oliva
2004-12-14 22:31     ` Richard Henderson
2004-12-15  6:56       ` Alexandre Oliva
2004-12-15  8:09         ` Richard Henderson
2004-12-15 20:07           ` Alexandre Oliva
2004-12-16 14:14             ` Dave Korn
2004-12-14 16:22 ` Bernd Schmidt
2004-12-14 20:53   ` Alexandre Oliva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).