Re: [patch, mips] Size savings for MIPS16 switch statements
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 sell...@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
Re: [patch, mips] Size savings for MIPS16 switch statements
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 sell...@mips.com 2013-07-30 Steve Ellcey sell...@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; } }
Re: [patch, mips] Size savings for MIPS16 switch statements
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
Re: [patch, mips] Size savings for MIPS16 switch statements
Steve Ellcey sell...@mips.com writes: 2013-07-30 Steve Ellcey sell...@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
Re: [patch, mips] Size savings for MIPS16 switch statements
Steve Ellcey sell...@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
