Re: [Patch] MIPS: Align local variables like global variables

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

Revisiting a thread from a month ago:

Richard Sandiford <rsandifo@nildram.co.uk> writes:
> Nigel Stephens <nigel@mips.com> writes:
>> +/* If defined, a C expression to compute the alignment for a local
>> +   variable.  TYPE is the data type, and ALIGN is the alignment that
>> +   the object would ordinarily have.  The value of this macro is used
>> +   instead of that alignment to align the object.
>>  
>> +   If this macro is not defined, then ALIGN is used.
>> +
>> +   One use of this macro is to increase alignment of medium-size data
>> +   to make it all fit in fewer cache lines.  We need this for the same
>> +   reason as DATA_ALIGNMENT, namely to cause character arrays to be
>> +   word-aligned so that `strcpy' calls that copy constants to
>> +   character arrays can be done inline, and 'strcmp' can be optimised
>> +   to use word loads. */
>> +
>> +#define LOCAL_ALIGNMENT(TYPE, ALIGN)				\
>> +  DATA_ALIGNMENT(TYPE, ALIGN)
>
> Nits first: please drop the boiler-plate stuff, i.e. everything up to
> "fewer cache lines."  (I know there are lots of pre-existing examples,
> but I'm hoping to make a sweep over them before 4.3.)  Missing space
> in "DATA_ALIGNMENT(".
>
> More importantly, is this a win even for -Os?  Unlike DATA_ALIGNMENT[*],
> this should be purely a tuning decision, so can depend on things like
> optimize_size.
>
> [*] ISTR MIPSpro expected global variables to have the alignment
>     specified by DATA_ALIGNMENT, even if gcc doesn't.

I wasn't sure either way on the -Os question, so I ran CSiBE for
mipsisa64-elfoabi.  The results were:

-Os -mips32r2                                  3538305  3538225 :  100.00%
-Os -mips32r2 -mips16                          2834133  2831557 :   99.91%
-Os -mips64                                    3519921  3519441 :   99.99%

That's a pretty clear win, so I went ahead and applied the patch below
after testing on mipsisa32-elf.

Richard


gcc/
2007-10-15  Nigel Stephens  <nigel@mips.com>

	* config/mips/mips.h (LOCAL_ALIGNMENT): Define.

Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 129366)
+++ gcc/config/mips/mips.h	(working copy)
@@ -1331,7 +1331,13 @@ #define DATA_ALIGNMENT(TYPE, ALIGN)					
 	|| TREE_CODE (TYPE) == UNION_TYPE				\
 	|| TREE_CODE (TYPE) == RECORD_TYPE)) ? BITS_PER_WORD : (ALIGN))
 
-
+/* We need this for the same reason as DATA_ALIGNMENT, namely to cause
+   character arrays to be word-aligned so that `strcpy' calls that copy
+   constants to character arrays can be done inline, and 'strcmp' can be
+   optimised to use word loads. */
+#define LOCAL_ALIGNMENT(TYPE, ALIGN) \
+  DATA_ALIGNMENT (TYPE, ALIGN)
+  
 #define PAD_VARARGS_DOWN \
   (FUNCTION_ARG_PADDING (TYPE_MODE (type), type) == downward)