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

[Steve Ellcey <sellcey at imgtec dot com>]

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.
 
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
@@ -79,7 +79,7 @@ public:
       /* This optimization has no affect if TARGET_NEWABI.   If optimize
          is not at least 1 then the data needed for the optimization is
          not available and nothing will be done anyway.  */
-      return TARGET_OLDABI && flag_frame_header_optimization;
+      return TARGET_OLDABI && flag_frame_header_optimization && optimize > 0;
     }
 
   virtual unsigned int execute (function *) { return frame_header_opt (); }
@@ -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.  */
+
+static bool
+has_inlined_assembly (function *fn)
+{
+  basic_block bb;
+  gimple_stmt_iterator gsi;
+
+  /* If we do not have a cfg for this function be conservative and assume
+     it is may have inline assembly.  */
+  if (fn->cfg == NULL)
+    return true;
+
+  FOR_EACH_BB_FN (bb, fn)
+    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+      if (gimple_code (gsi_stmt (gsi)) == GIMPLE_ASM)
+	return true;
+
+  return false;
+}
+
 /* Return true if this function will use the stack space allocated by its
    caller or if we cannot determine for certain that it does not.  */
 
@@ -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. */
+      if (DECL_MODE (t) != SImode)
+	return true;
     }
 
   return false;
 }
 
-/* Returns TRUE if the argument stack space allocated by function FN is used.
-   Returns FALSE if the space is needed or if the need for the space cannot
+/* Return true if the argument stack space allocated by function FN is used.
+   Return false if the space is needed or if the need for the space cannot
    be determined.  */
 
 static bool
@@ -177,6 +206,8 @@ callees_functions_use_frame_header (function *fn)
 	          called_fn = DECL_STRUCT_FUNCTION (called_fn_tree);
 		  if (called_fn == NULL
 		      || DECL_WEAK (called_fn_tree) 
+		      || has_inlined_assembly (called_fn)
+		      || !is_leaf_function (called_fn)
 		      || !called_fn->machine->does_not_use_frame_header)
 		    return true;
 	        }
@@ -188,6 +219,41 @@ callees_functions_use_frame_header (function *fn)
   return false;
 }
 
+/* Set the callers_may_not_allocate_frame flag for any function which
+   function FN calls because FN may not allocate a frame header.  */
+
+static void
+set_callers_may_not_allocate_frame (function *fn)
+{
+  basic_block bb;
+  gimple_stmt_iterator gsi;
+  gimple *stmt;
+  tree called_fn_tree;
+  function *called_fn;
+
+  if (fn->cfg == NULL)
+    return;
+
+  FOR_EACH_BB_FN (bb, fn)
+    {
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	{
+	  stmt = gsi_stmt (gsi);
+	  if (is_gimple_call (stmt))
+	    {
+	      called_fn_tree = gimple_call_fndecl (stmt);
+	      if (called_fn_tree != NULL)
+	        {
+	          called_fn = DECL_STRUCT_FUNCTION (called_fn_tree);
+		  if (called_fn != NULL)
+		    called_fn->machine->callers_may_not_allocate_frame = true;
+	        }
+            }
+        }
+    }
+  return;
+}
+
 /* Scan each function to determine those that need its frame headers.  Perform
    a second scan to determine if the allocation can be skipped because none of
    their callees require the frame header.  */
@@ -209,8 +275,16 @@ frame_header_opt ()
     {
       fn = node->get_fun ();
       if (fn != NULL)
-        fn->machine->optimize_call_stack
-	  = !callees_functions_use_frame_header (fn);
+	fn->machine->optimize_call_stack
+	  = !callees_functions_use_frame_header (fn) && !is_leaf_function (fn);
     }
+
+  FOR_EACH_DEFINED_FUNCTION (node)
+    {
+      fn = node->get_fun ();
+      if (fn != NULL && fn->machine->optimize_call_stack)
+	set_callers_may_not_allocate_frame (fn);
+    }
+
   return 0;
 }
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.  */
+
+  if (TARGET_OLDABI
+      && optimize > 0
+      && flag_frame_header_optimization
+      && !MAIN_NAME_P (DECL_NAME (current_function_decl))
+      && cfun->machine->varargs_size == 0
+      && crtl->args.pretend_args_size == 0
+      && frame->var_size == 0
+      && frame->num_acc == 0
+      && frame->num_cop0_regs == 0
+      && frame->num_fp == 0
+      && frame->num_gp > 0
+      && frame->num_gp <= MAX_ARGS_IN_REGISTERS
+      && !GENERATE_MIPS16E_SAVE_RESTORE
+      && !cfun->machine->interrupt_handler_p
+      && cfun->machine->does_not_use_frame_header
+      && cfun->machine->optimize_call_stack
+      && !cfun->machine->callers_may_not_allocate_frame
+      && !mips_cfun_has_cprestore_slot_p ())
+    {
+      offset = 0;
+      frame->gp_sp_offset = REG_PARM_STACK_SPACE(cfun) - UNITS_PER_WORD;
+      cfun->machine->use_frame_header_for_callee_saved_regs = true;
+    }
+
   /* Move above the callee-allocated varargs save area.  */
   offset += MIPS_STACK_ALIGN (cfun->machine->varargs_size);
   frame->arg_pointer_offset = offset;
@@ -11583,12 +11612,15 @@ mips_expand_prologue (void)
 	    }
 	  else
 	    {
-	      rtx insn = gen_add3_insn (stack_pointer_rtx,
-					stack_pointer_rtx,
-					GEN_INT (-step1));
-	      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
-	      mips_frame_barrier ();
-	      size -= step1;
+	      if (step1 != 0)
+		{
+		  rtx insn = gen_add3_insn (stack_pointer_rtx,
+					    stack_pointer_rtx,
+					    GEN_INT (-step1));
+		  RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
+		  mips_frame_barrier ();
+		  size -= step1;
+		}
 	    }
 	  mips_for_each_saved_acc (size, mips_save_reg);
 	  mips_for_each_saved_gpr_and_fpr (size, mips_save_reg);
@@ -11713,9 +11745,9 @@ mips_epilogue_emit_cfa_restores (void)
   rtx_insn *insn;
 
   insn = get_last_insn ();
-  gcc_assert (insn && !REG_NOTES (insn));
   if (mips_epilogue.cfa_restores)
     {
+      gcc_assert (insn && !REG_NOTES (insn));
       RTX_FRAME_RELATED_P (insn) = 1;
       REG_NOTES (insn) = mips_epilogue.cfa_restores;
       mips_epilogue.cfa_restores = 0;
@@ -11966,7 +11998,9 @@ mips_expand_epilogue (bool sibcall_p)
 	mips_deallocate_stack (stack_pointer_rtx, GEN_INT (step2), 0);
     }
 
-  if (!use_jraddiusp_p)
+  if (cfun->machine->use_frame_header_for_callee_saved_regs)
+    mips_epilogue_emit_cfa_restores ();
+  else if (!use_jraddiusp_p)
     gcc_assert (!mips_epilogue.cfa_restores);
 
   /* Add in the __builtin_eh_return stack adjustment.  We need to
@@ -12068,7 +12102,8 @@ mips_can_use_return_insn (void)
   if (mips16_cfun_returns_in_fpr_p ())
     return false;
 
-  return cfun->machine->frame.total_size == 0;
+  return (cfun->machine->frame.total_size == 0
+	  && !cfun->machine->use_frame_header_for_callee_saved_regs);
 }
 
 /* Return true if register REGNO can store a value of mode MODE.
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 501d283..7a4a0ba 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -3273,6 +3273,13 @@ struct GTY(())  machine_function {
   /* True if none of the functions that are called by this function need
      stack space allocated for their arguments.  */
   bool optimize_call_stack;
+
+  /* True if one of the functions calling this function may not allocate
+     a frame header.  */
+  bool callers_may_not_allocate_frame;
+
+  /* True if GCC stored callee saved registers in the frame header.  */
+  bool use_frame_header_for_callee_saved_regs;
 };
 #endif
 


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);
+}
+