Patchwork [mips] Size savings for MIPS16 switch statements

login
register
mail settings
Submitter Steve Ellcey
Date July 23, 2013, 5:25 p.m.
Message ID <b1eb2749-d413-4f7f-9bb5-42e33787cd02@BAMAIL02.ba.imgtec.org>
Download mbox | patch
Permalink /patch/261163/
State New
Headers show

Comments

Steve Ellcey - July 23, 2013, 5:25 p.m.
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.
Richard Sandiford - July 23, 2013, 6:43 p.m.
"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
Maciej W. Rozycki - July 30, 2013, 10:18 a.m.
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

Patch

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 ();
+}
 
 /* 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;
 
 #include "gt-mips.h"