public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch, mips] Size savings for MIPS16 switch statements
@ 2013-07-23 17:56 Steve Ellcey 
  2013-07-23 19:48 ` Richard Sandiford
  2013-07-30 11:41 ` Maciej W. Rozycki
  0 siblings, 2 replies; 6+ messages in thread
From: Steve Ellcey  @ 2013-07-23 17:56 UTC (permalink / raw)
  To: rdsandiford, gcc-patches

While doing some space optimization work with mips16 I found that using a
larger case threshold value could shrink the code.  I did testing on some
libraries like libpng and libjpeg as well as some test cases I wrote and
came up with 10 as the best value for space savings in mips16 mode.  I did
some testing of mips32 code as well and found that this change did not
help with that code so I restricted the change to mips16 only.

Tested on mips-mti-elf target, OK for checkin?

Steve Ellcey
sellcey@mips.com



2013-07-23  Steve Ellcey  <sellcey@mips.com>

	* config/mips/mips.c (mips_case_values_threshold): New.
	(TARGET_CASE_VALUES_THRESHOLD): Define.



diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index a3735dc..fb39f7c 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -18613,6 +18613,19 @@ mips_expand_vec_minmax (rtx target, rtx op0, rtx op1,
   x = gen_rtx_IOR (vmode, t0, t1);
   emit_insn (gen_rtx_SET (VOIDmode, target, x));
 }
+
+/* Implement `CASE_VALUES_THRESHOLD'.  */
+/* Supply the default for --param case-values-threshold=0  */
+
+unsigned int
+mips_case_values_threshold (void)
+{
+  /* In MIPS16 mode using a larger case threshold generates smaller code.  */
+  if (TARGET_MIPS16 && optimize_size)
+    return 10;
+  else
+    return default_case_values_threshold ();
+}
 \f
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
@@ -18844,6 +18857,9 @@ mips_expand_vec_minmax (rtx target, rtx op0, rtx op1,
 #undef TARGET_VECTORIZE_VEC_PERM_CONST_OK
 #define TARGET_VECTORIZE_VEC_PERM_CONST_OK mips_vectorize_vec_perm_const_ok
 
+#undef TARGET_CASE_VALUES_THRESHOLD
+#define TARGET_CASE_VALUES_THRESHOLD mips_case_values_threshold
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 #include "gt-mips.h"

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

* Re: [patch, mips] Size savings for MIPS16 switch statements
  2013-07-23 17:56 [patch, mips] Size savings for MIPS16 switch statements Steve Ellcey 
@ 2013-07-23 19:48 ` Richard Sandiford
  2013-07-30 11:41 ` Maciej W. Rozycki
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Sandiford @ 2013-07-23 19:48 UTC (permalink / raw)
  To: Steve Ellcey ; +Cc: gcc-patches

"Steve Ellcey " <sellcey@mips.com> writes:
> While doing some space optimization work with mips16 I found that using a
> larger case threshold value could shrink the code.  I did testing on some
> libraries like libpng and libjpeg as well as some test cases I wrote and
> came up with 10 as the best value for space savings in mips16 mode.  I did
> some testing of mips32 code as well and found that this change did not
> help with that code so I restricted the change to mips16 only.

Thanks for doing this.  casesi certainly isn't small, so I can believe
a larger threshold makes sense.  OK with a minor change:

> +/* Implement `CASE_VALUES_THRESHOLD'.  */
> +/* Supply the default for --param case-values-threshold=0  */
> +
> +unsigned int

Please just use:

/* Implement TARGET_CASE_VALUES_THRESHOLD.  */

instead of these two comments.

I was worried whether this would work for mips16 attributes, but it looks
like the function is called on demand rather than cached, so there should
be no problem there.

Thanks,
Richard

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

* Re: [patch, mips] Size savings for MIPS16 switch statements
  2013-07-23 17:56 [patch, mips] Size savings for MIPS16 switch statements Steve Ellcey 
  2013-07-23 19:48 ` Richard Sandiford
@ 2013-07-30 11:41 ` Maciej W. Rozycki
  2013-07-30 16:56   ` Steve Ellcey
  1 sibling, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2013-07-30 11:41 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: Richard Sandiford, gcc-patches

On Tue, 23 Jul 2013, Steve Ellcey  wrote:

> While doing some space optimization work with mips16 I found that using a
> larger case threshold value could shrink the code.  I did testing on some
> libraries like libpng and libjpeg as well as some test cases I wrote and
> came up with 10 as the best value for space savings in mips16 mode.  I did
> some testing of mips32 code as well and found that this change did not
> help with that code so I restricted the change to mips16 only.
> 
> Tested on mips-mti-elf target, OK for checkin?
> 
> 2013-07-23  Steve Ellcey  <sellcey@mips.com>
> 
> 	* config/mips/mips.c (mips_case_values_threshold): New.
> 	(TARGET_CASE_VALUES_THRESHOLD): Define.

 This change has caused regressions I believe, with the mips-linux-gnu 
target and the MIPS32/o32 multilib:

FAIL: gcc.target/mips/code-readable-1.c  -Os   scan-assembler \tla\t
FAIL: gcc.target/mips/code-readable-1.c  -Os   scan-assembler \t\\.half\t
FAIL: gcc.target/mips/code-readable-2.c  -Os   scan-assembler \t\\.word\t[^\n]*L
FAIL: gcc.target/mips/code-readable-3.c  -Os   scan-assembler %hi\\([^)]*L
FAIL: gcc.target/mips/code-readable-3.c  -Os   scan-assembler %lo\\([^)]*L
FAIL: gcc.target/mips/code-readable-4.c  -Os   scan-assembler \t\\.half\t
FAIL: gcc.target/mips/code-readable-4.c  -Os   scan-assembler \tla\t

-- it may be that the tests have to be disabled at -Os just like e.g. 
code-readable-1.c already is at -O0.

  Maciej

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

* Re: [patch, mips] Size savings for MIPS16 switch statements
  2013-07-30 11:41 ` Maciej W. Rozycki
@ 2013-07-30 16:56   ` Steve Ellcey
  2013-07-30 18:47     ` Maciej W. Rozycki
  2013-07-30 19:36     ` Richard Sandiford
  0 siblings, 2 replies; 6+ messages in thread
From: Steve Ellcey @ 2013-07-30 16:56 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Richard Sandiford, gcc-patches

On Tue, 2013-07-30 at 11:18 +0100, Maciej W. Rozycki wrote:
> 
> -- it may be that the tests have to be disabled at -Os just like e.g. 
> code-readable-1.c already is at -O0.
> 
>   Maciej

Sorry about that, not sure why I didn't notice the failures.  Rather
then skipping the tests for -Os I was thinking it might be better to
increase the size of the switch statements.  Here is a patch I have
tested to fix these failures.

Richard, does this look OK for checkin?

Steve Ellcey
sellcey@mips.com


2013-07-30  Steve Ellcey  <sellcey@mips.com>

	* gcc.target/mips/code-readable-1.c: Increase switch size.
	* gcc.target/mips/code-readable-2.c: Ditto.
	* gcc.target/mips/code-readable-3.c: Ditto.
	* gcc.target/mips/code-readable-4.c: Ditto.

diff --git a/gcc/testsuite/gcc.target/mips/code-readable-1.c b/gcc/testsuite/gcc.target/mips/code-readable-1.c
index 34c2c0a..b3e864d 100644
--- a/gcc/testsuite/gcc.target/mips/code-readable-1.c
+++ b/gcc/testsuite/gcc.target/mips/code-readable-1.c
@@ -8,6 +8,10 @@ volatile int x4;
 volatile int x5;
 volatile int x6;
 volatile int x7;
+volatile int x8;
+volatile int x9;
+volatile int x10;
+volatile int x11;
 
 MIPS16 int
 foo (int i, volatile *x)
@@ -21,6 +25,10 @@ foo (int i, volatile *x)
     case 5: return x5 + x[4];
     case 6: return x6 + x[5];
     case 7: return x7 + x[6];
+    case 8: return x8 + x[7];
+    case 9: return x9 + x[8];
+    case 10: return x10 + x[9];
+    case 11: return x11 + x[10];
     default: return 0;
     }
 }
diff --git a/gcc/testsuite/gcc.target/mips/code-readable-2.c b/gcc/testsuite/gcc.target/mips/code-readable-2.c
index 71aeb13..3d32504 100644
--- a/gcc/testsuite/gcc.target/mips/code-readable-2.c
+++ b/gcc/testsuite/gcc.target/mips/code-readable-2.c
@@ -7,6 +7,10 @@ volatile int x4;
 volatile int x5;
 volatile int x6;
 volatile int x7;
+volatile int x8;
+volatile int x9;
+volatile int x10;
+volatile int x11;
 
 MIPS16 int
 foo (int i, volatile *x)
@@ -20,6 +24,10 @@ foo (int i, volatile *x)
     case 5: return x5 + x[4];
     case 6: return x6 + x[5];
     case 7: return x7 + x[6];
+    case 8: return x8 + x[7];
+    case 9: return x9 + x[8];
+    case 10: return x10 + x[9];
+    case 11: return x11 + x[10];
     default: return 0;
     }
 }
diff --git a/gcc/testsuite/gcc.target/mips/code-readable-3.c b/gcc/testsuite/gcc.target/mips/code-readable-3.c
index fc78505..aaf1874 100644
--- a/gcc/testsuite/gcc.target/mips/code-readable-3.c
+++ b/gcc/testsuite/gcc.target/mips/code-readable-3.c
@@ -7,6 +7,10 @@ volatile int x4;
 volatile int x5;
 volatile int x6;
 volatile int x7;
+volatile int x8;
+volatile int x9;
+volatile int x10;
+volatile int x11;
 
 MIPS16 int
 foo (int i, volatile *x)
@@ -20,6 +24,10 @@ foo (int i, volatile *x)
     case 5: return x5 + x[4];
     case 6: return x6 + x[5];
     case 7: return x7 + x[6];
+    case 8: return x8 + x[7];
+    case 9: return x9 + x[8];
+    case 10: return x10 + x[9];
+    case 11: return x11 + x[10];
     default: return 0;
     }
 }
diff --git a/gcc/testsuite/gcc.target/mips/code-readable-4.c b/gcc/testsuite/gcc.target/mips/code-readable-4.c
index ae8ff8a..4db89f8 100644
--- a/gcc/testsuite/gcc.target/mips/code-readable-4.c
+++ b/gcc/testsuite/gcc.target/mips/code-readable-4.c
@@ -8,6 +8,10 @@ volatile int x4;
 volatile int x5;
 volatile int x6;
 volatile int x7;
+volatile int x8;
+volatile int x9;
+volatile int x10;
+volatile int x11;
 
 MIPS16 int
 foo (int i, volatile *x)
@@ -21,6 +25,10 @@ foo (int i, volatile *x)
     case 5: return x5 + x[4];
     case 6: return x6 + x[5];
     case 7: return x7 + x[6];
+    case 8: return x8 + x[7];
+    case 9: return x9 + x[8];
+    case 10: return x10 + x[9];
+    case 11: return x11 + x[10];
     default: return 0;
     }
 }




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

* Re: [patch, mips] Size savings for MIPS16 switch statements
  2013-07-30 16:56   ` Steve Ellcey
@ 2013-07-30 18:47     ` Maciej W. Rozycki
  2013-07-30 19:36     ` Richard Sandiford
  1 sibling, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2013-07-30 18:47 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: Richard Sandiford, gcc-patches

On Tue, 30 Jul 2013, Steve Ellcey wrote:

> > -- it may be that the tests have to be disabled at -Os just like e.g. 
> > code-readable-1.c already is at -O0.
> 
> Sorry about that, not sure why I didn't notice the failures.  Rather
> then skipping the tests for -Os I was thinking it might be better to
> increase the size of the switch statements.  Here is a patch I have
> tested to fix these failures.

 That sounds even better to me -- perhaps you can take the opportunity and 
test the new threshold too?  I.e. another test case for MIPS16 and -Os 
only that makes sure the code produced is switched between the two models 
when the size of a switch statement crosses the boundary?

  Maciej

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

* Re: [patch, mips] Size savings for MIPS16 switch statements
  2013-07-30 16:56   ` Steve Ellcey
  2013-07-30 18:47     ` Maciej W. Rozycki
@ 2013-07-30 19:36     ` Richard Sandiford
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Sandiford @ 2013-07-30 19:36 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: Maciej W. Rozycki, gcc-patches

Steve Ellcey <sellcey@mips.com> writes:
> 2013-07-30  Steve Ellcey  <sellcey@mips.com>
>
> 	* gcc.target/mips/code-readable-1.c: Increase switch size.
> 	* gcc.target/mips/code-readable-2.c: Ditto.
> 	* gcc.target/mips/code-readable-3.c: Ditto.
> 	* gcc.target/mips/code-readable-4.c: Ditto.

OK, thanks.

Richard

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

end of thread, other threads:[~2013-07-30 19:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-23 17:56 [patch, mips] Size savings for MIPS16 switch statements Steve Ellcey 
2013-07-23 19:48 ` Richard Sandiford
2013-07-30 11:41 ` Maciej W. Rozycki
2013-07-30 16:56   ` Steve Ellcey
2013-07-30 18:47     ` Maciej W. Rozycki
2013-07-30 19:36     ` Richard Sandiford

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