RE: [Patch, MIPS] Frame header optimization for MIPS (part 2)

["Moore, Catherine" <Catherine_Moore@mentor.com>]

> -----Original Message-----
> From: Steve Ellcey [mailto:sellcey@imgtec.com]
> Sent: Friday, October 23, 2015 2:08 PM
> To: Myers, Joseph
> Cc: Bernd Schmidt; Mike Stump; gcc-patches@gcc.gnu.org;
> matthew.fortune@imgtec.com; Moore, Catherine
> Subject: Re: [Patch, MIPS] Frame header optimization for MIPS (part 2)
> 
> Just to follow up on this string, here is a new version of the patch
> with the extraneous parenthesis removed.
> 
> Steve Ellcey
> sellcey@imgtec.com
> 
> 
> 2015-10-23  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* frame-header-opt.c (gate): Check for optimize > 0.
> 	(has_inlined_assembly): New function.
> 	(needs_frame_header_p): Remove is_leaf_function check,
> 	add argument type check.
> 	(callees_functions_use_frame_header): Add is_leaf_function
> 	and has_inlined_assembly calls..
> 	(set_callers_may_not_allocate_frame): New function.
> 	(frame_header_opt): Add is_leaf_function call, add
> 	set_callers_may_not_allocate_frame call.
> 	* config/mips/mips.c (mips_compute_frame_info): Add check
> 	to see if callee saved regs can be put in frame header.
> 	(mips_expand_prologue): Add check to see if step1 is zero,
> 	fix cfa restores when using frame header to store regs.
> 	(mips_can_use_return_insn): Check to see if registers are
> 	stored in frame header.
> 	* config/mips/mips.h (machine_function): Add
> 	callers_may_not_allocate_frame and
> 	use_frame_header_for_callee_saved_regs fields.

This is okay after making the minor corrections embedded below.
I'm sorry that it took me so long to review this patch.
Catherine

> 
> diff --git a/gcc/config/mips/frame-header-opt.c b/gcc/config/mips/frame-
> header-opt.c
> index 7c7b1f2..2cf589d 100644
> --- a/gcc/config/mips/frame-header-opt.c
> +++ b/gcc/config/mips/frame-header-opt.c
> @@ -125,6 +125,29 @@ is_leaf_function (function *fn)
>    return true;
>  }
> 
> +/* Return true if this function has inline assembly code or if we cannot
> +   be certain that it does not.  False if know that there is no inline
> +   assembly.  */
> +

 s/False if know/False if we know/

> @@ -136,20 +159,26 @@ needs_frame_header_p (function *fn)
>    if (fn->decl == NULL)
>      return true;
> 
> -  if (fn->stdarg || !is_leaf_function (fn))
> +  if (fn->stdarg)
>      return true;
> 
>    for (t = DECL_ARGUMENTS (fn->decl); t; t = TREE_CHAIN (t))
>      {
>        if (!use_register_for_decl (t))
> -	  return true;
> +	return true;
> +
> +      /* Some 64 bit types may get copied to general registers using the frame
> +	 header, see mips_output_64bit_xfer.  Checking for SImode only may be
> +         overly restrictive but it is gauranteed to be safe. */

  s/64 bit/64-bit/
 s/gauranteed/guaranteed/

> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index c5affc8..5a9d48d 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -10465,6 +10465,35 @@ mips_compute_frame_info (void)
>        frame->cop0_sp_offset = offset - UNITS_PER_WORD;
>      }
> 
> +  /* Determine if we can save the callee saved registers in the frame
> +     header.  Restrict this to functions where there is no other reason
> +     to allocate stack space so that we can completely eliminate the
> +     instructions that modify the stack pointer.  */
> +
  s/callee saved/callee-saved/
  s/completely//

> 
> 
> 
> 2015-10-23  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* gcc.target/mips/frame-header-4.c: New test.
> 
> diff --git a/gcc/testsuite/gcc.target/mips/frame-header-4.c
> b/gcc/testsuite/gcc.target/mips/frame-header-4.c
> index e69de29..3cddba1 100644
> --- a/gcc/testsuite/gcc.target/mips/frame-header-4.c
> +++ b/gcc/testsuite/gcc.target/mips/frame-header-4.c
> @@ -0,0 +1,20 @@
> +/* Verify that we can optimize away the frame header allocation in bar
> +   by having it use its frame header to store $31 in before calling foo.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mframe-header-opt -mabi=32" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler-not "\taddiu\t\\\$sp" } } */
> +
> +int __attribute__ ((noinline))
> +foo (int a, int b)
> +{
> +	return a + b;
> +}
> +
> +int  __attribute__ ((noinline))
> +bar (int a, int b)
> +{
> +	return 1 + foo(a,b);
> +}
> +

  Space after foo, please.  Change the two tabs to two space each.