Patchwork [mips] Size optimization for MIPS

login
register
mail settings
Submitter Steve Ellcey
Date July 9, 2013, 10:15 p.m.
Message ID <1373408109.5932.28.camel@ubuntu-sellcey>
Download mbox | patch
Permalink /patch/257871/
State New
Headers show

Comments

Steve Ellcey - July 9, 2013, 10:15 p.m.
On Tue, 2013-07-09 at 18:25 +0100, Richard Sandiford wrote:

> 
> That was always the case though.  These registers weren't enabled because
> you can do operations on them.  They were enabled because they should make
> ideal spill space.  Moves to and from these registers always take 2 bytes,
> whereas stack spills take either 2 or 4 bytes (as well as being slower).
> 
> So it sounds like the problem is that the heuristics aren't tuned properly.
> Disabling the registers seems like papering over the problem rather than
> fixing it.  But since no-one is likely to do the work to fix the heuristics,
> that isn't a good enough reason to reject the patch.

Pre-LRA I don't think there was any good way to tell GCC to use a
register for reloads but not for anything else.  I think LRA does
support this and someone here has started looking at LRA but we haven't
come up with register class definitions that make LRA work better then
the existing setup (particularly in terms of MIPS16 object size).  Do
you know if anyone else has started looking at LRA on MIPS?

> Please combine this with the previous block though, since the comment here
> contradicts the comment there.  It would also be worth saying why you
> keep $24 (for CMP and CMPI) and $25 (for SVR4 PIC).

OK, I have done this (see attached new patch).

> Please also run some sanity checks for -mabi=eabi and functions that
> have 5+ arguments (all used) to make sure that they still work.
> 
> Thanks,
> Richard

I built a MIPS GCC with newlib that could handle -mabi=eabi and ran the
GCC testsuite, it all looked good.  I am pretty sure the GCC testsuite
has tests with 5+ arguments but I did a couple of hand tests as well and
they also looked good.

Steve Ellcey
sellcey@mips.com


Steve Ellcey  <sellcey@mips.com>

	* config/mips/mips.c (mips_conditional_register_usage): Do not
	use t[0-7] registers in MIPS16 mode when optimizing for size.
Richard Sandiford - July 11, 2013, 7:50 p.m.
Steve Ellcey <sellcey@mips.com> writes:
> On Tue, 2013-07-09 at 18:25 +0100, Richard Sandiford wrote:
>> That was always the case though.  These registers weren't enabled because
>> you can do operations on them.  They were enabled because they should make
>> ideal spill space.  Moves to and from these registers always take 2 bytes,
>> whereas stack spills take either 2 or 4 bytes (as well as being slower).
>> 
>> So it sounds like the problem is that the heuristics aren't tuned properly.
>> Disabling the registers seems like papering over the problem rather than
>> fixing it.  But since no-one is likely to do the work to fix the heuristics,
>> that isn't a good enough reason to reject the patch.
>
> Pre-LRA I don't think there was any good way to tell GCC to use a
> register for reloads but not for anything else.  I think LRA does
> support this and someone here has started looking at LRA but we haven't
> come up with register class definitions that make LRA work better then
> the existing setup (particularly in terms of MIPS16 object size).  Do
> you know if anyone else has started looking at LRA on MIPS?

No.  I should be doing it myself, sorry, but I've got distracted by
binutils stuff recently.  Glad to hear someone's looking at it.

It isn't necessarily just classes that need to change though.
Things like the register move costs might need tweaking too.

> +	 and $25 (t9) because it is used as the SVR4 PIC register.  */

...as the function call address in SVR4 PIC.

Also, please make sure that the comment consistently uses tabs (sorry).
Some lines were indented purely with spaces.

OK with that change, thanks.

Richard

Patch

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bd1d10b..4b5a62c 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -17199,10 +17199,16 @@  mips_conditional_register_usage (void)
     }
   if (TARGET_MIPS16)
     {
-      /* In MIPS16 mode, we permit the $t temporary registers to be used
-	 for reload.  We prohibit the unused $s registers, since they
+      /* In MIPS16 mode, we prohibit the unused $s registers, since they
 	 are call-saved, and saving them via a MIPS16 register would
-	 probably waste more time than just reloading the value.  */
+         probably waste more time than just reloading the value.
+
+         We permit the $t temporary registers when optimizing for speed
+	 but not when optimizing for space because using them results in
+	 code that is larger (but faster) then not using them.  We do
+	 allow $24 (t8) because it is used in CMP and CMPI instructions
+	 and $25 (t9) because it is used as the SVR4 PIC register.  */
+
       fixed_regs[18] = call_used_regs[18] = 1;
       fixed_regs[19] = call_used_regs[19] = 1;
       fixed_regs[20] = call_used_regs[20] = 1;
@@ -17212,6 +17218,17 @@  mips_conditional_register_usage (void)
       fixed_regs[26] = call_used_regs[26] = 1;
       fixed_regs[27] = call_used_regs[27] = 1;
       fixed_regs[30] = call_used_regs[30] = 1;
+      if (optimize_size)
+	{
+	  fixed_regs[8] = call_used_regs[8] = 1;
+	  fixed_regs[9] = call_used_regs[9] = 1;
+	  fixed_regs[10] = call_used_regs[10] = 1;
+	  fixed_regs[11] = call_used_regs[11] = 1;
+	  fixed_regs[12] = call_used_regs[12] = 1;
+	  fixed_regs[13] = call_used_regs[13] = 1;
+	  fixed_regs[14] = call_used_regs[14] = 1;
+	  fixed_regs[15] = call_used_regs[15] = 1;
+	}
 
       /* Do not allow HI and LO to be treated as register operands.
 	 There are no MTHI or MTLO instructions (or any real need