Re: [PATCH][MIPS] Add -mgrow-frame-downwards option

[Sandra Loosemore <sandra at codesourcery dot com>]

On 05/20/2016 08:58 AM, Robert Suchanek wrote:
> Hi,
>
> The patch changes the default behaviour of the direction in which
> the local frame grows for MIPS16.
>
> The code size reduces by about 0.5% in average case for -Os, hence,
> it is good to turn the option on by default.
>
> Ok to apply?
>
> Regards,
> Robert
>
> gcc/
>
> 2016-05-20  Matthew Fortune  <matthew.fortune@imgtec.com>
>
> 	* config/mips/mips.h (FRAME_GROWS_DOWNWARD): Enable it
> 	conditionally for MIPS16.
> 	* config/mips/mips.opt: Add -mgrow-frame-downwards option.
> 	Enable it by default for MIPS16.
> 	* doc/invoke.texi: Document the option.

This may be a stupid question, but what point is there in exposing this 
as an option to users?  Users generally just want the compiler to emit 
good code when they compile with -O, not more individual optimization 
switches to twiddle.  Is FRAME_GROWS_DOWNWARD likely to be so buggy or 
poorly tested that it's necessary to provide a way to turn it off?

If we really must have this option....

> diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
> index 3b92ef5..53feb23 100644
> --- a/gcc/config/mips/mips.opt
> +++ b/gcc/config/mips/mips.opt
> @@ -447,3 +447,7 @@ Enum(mips_cb_setting) String(always) Value(MIPS_CB_ALWAYS)
>   minline-intermix
>   Target Report Var(TARGET_INLINE_INTERMIX)
>   Allow inlining even if the compression flags differ between caller and callee.
> +
> +mgrow-frame-downwards
> +Target Report Var(TARGET_FRAME_GROWS_DOWNWARDS) Init(1)
> +Change the behaviour to grow the frame downwards for MIPS16.

British spelling of "behaviour" here.  How about just "Grow the frame 
downwards for MIPS16."

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 2f6195e..6e5d620 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -17929,6 +17930,18 @@ vice-versa.  When using this option it is necessary to protect functions
>   that cannot be compiled as MIPS16 with a @code{noinline} attribute to ensure
>   they are not inlined into a MIPS16 function.
>
> +@item -mgrow-frame-downwards
> +@itemx -mno-grow-frame-downwards
> +@opindex mgrow-frame-downwards
> +Grow the local frame down (up) for MIPS16.
> +
> +Growing the frame downwards allows us to get spill slots created at the lowest

s/allows us to get spill slots created/allows GCC to create spill slots/

> +address rather than the highest address in a local frame.  The benefit of this
> +is smaller code size as accessing spill splots closer to the stack pointer
> +can be done using using 16-bit instructions.

s/spill splots/spill slots/

But, this option description is so implementor-speaky that it just 
reinforces my thinking that it's likely to be uninteresting to users....

> +
> +The option is enabled by default (to grow frame downwards) for MIPS16.
> +
>   @item -mabi=32
>   @itemx -mabi=o64
>   @itemx -mabi=n32

-Sandra