[PATCH, MIPS] fix MIPS16 hard-float function stub bugs

[Sandra Loosemore <sandra at codesourcery dot com>]

This patch fixes a group of bugs that were causing link errors on 
hard-float MIPS16 code built with a mips-linux-gnu toolchain.  This is 
Mark Mitchell's analysis of the original problem:

> The MIPS16
> instruction set cannot directly access hard-float registers, so helper
> functions in libgcc are used to access floating-point registers.  In
> addition, the compiler handles functions which take floating-point
> values as arguments specially.  It generates a MIPS16 version of the
> function that expects the floating-point arguments in general-purpose
> registers.  But, since the hard-float ABI requires that floating-point
> arguments are passed in floating-point registers, the compiler also
> generates a MIPS32 stub which moves the floating-point values into
> general-purpose registers and then tail-calls the MIPS16 function.
> MIPS32 functions call the MIPS32 stub, while MIPS16 functions call the
> MIPS16 version of the function.
>
> The problem reported in the issue is an undefined symbol at link-time.
> The undefined symbol is *not* one of the libgcc helper functions.  It is
> also *not* the name of a MIPS32->MIPS16 stub function.  It's a third
> category: a local alias for the MIPS16 function.  When generating a
> stub, the compiler also generates a local (i.e., not visible outside the
> object module) alias for the main function (not the stub).
>
> So, for a function "f" with floating-point arguments, we end up with:
>
> (a) "f" itself, a MIPS16 function, expecting floating-point arguments in
> general-purpose registers
>
> (b) "__fn_stub_f", a MIPS32 function, expecting floating-point arguments
> in floating-point registers.  This function moves the arguments into
> general-purpose registers and then tail-calls "f".
>
> (c) "__fn_local_f", a local alias.
>
> The purpose of the alias is as follows.  When making an indirect call
> (i.e., a call via a function-pointer) to a function which either takes
> floating-point arguments or returns a floating-point value from MIPS16
> code, we must use a helper function in libgcc.  The helper function
> moves values to/from the floating-point registers to conform to the
> normal MIPS32 ABI.  But, if we're in MIPS16 code, and calling another
> MIPS16 function indirectly, there's no point to shuffle things in and
> out of floating-point registers.  In that case, we can skip the helper
> function and just directly call the MIPS16 function.  And, if we're
> calling from within the same object file, we don't want to use the name
> "f" -- that will cause the linker to generate extra goop that we don't
> need in the final executable.  So, we optimize by calling via the local
> alias.
>
> But, herein lies the bug.  We're trying to use the alias when we have a
> function that returns a floating-point value -- even though it doesn't
> have any floating-point arguments (see mips16_build_call_stub).  But, we
> only generate stubs for functions that have floating-point arguments
> (see mips16_build_function_stub) -- not for functions that have
> floating-point return values.  And we generate aliases as the same time
> that we generate stubs.  So, we never generate the alias.

So, the fix for this is to generate a local alias (but not a stub) for 
functions that return floating-point values, even if they do not have 
floating-point arguments.

In addition to that, there were still some related problems that had to 
be fixed to get this to work right:

* mips16_local_function_p, which is called from mips16_build_call_stub 
to detect the situations where we can optimize local calls, wasn't 
checking for weak or linkonce definitions, where the final copy of the 
function selected by the linker might come from a different object.

* The stubs for comdat functions need to go in the same comdat group as 
the function.

* The local stub symbol names should use the conventions for local 
symbol names.

We've had these fixes in our local code base for some time, and I 
regression-tested on a mips-linux-gnu mainline build.  OK to check in?

-Sandra


2012-08-07  Mark Mitchell <mark@codesourcery.com>
	    Maciej W. Rozycki  <macro@codesourcery.com>
	    Julian Brown  <julian@codesourcery.com>

	gcc/
	* config/mips/mips.c (mips16_local_function_p): Also reject weak
	and linkonce definitions.
	(mips16_local_alias): Use LOCAL_LABEL_PREFIX for the symbol
	generated.
	(mips16_build_function_stub): Split out the alias creation part.
	Declare stub once-only if target function is.
	(mips16_build_function_stub_and_local_alias): New function to do
	both things.
	(mips16_build_call_stub): Declare stub once-only if target
	function is.
	(mips_output_function_prologue):  Also check for cases where a
	a local alias only is needed to handle a floating-point return
	value.

	gcc/testsuite/
	* gcc.target/mips/mips.exp (mips_option_groups): Add
	interlink-mips16.
	* gcc.target/mips/mips16-fn-local.c: New test.


Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 190188)
+++ gcc/config/mips/mips.c	(working copy)
@@ -1530,6 +1530,8 @@ mips16_local_function_p (const_rtx x)
   return (GET_CODE (x) == SYMBOL_REF
 	  && SYMBOL_REF_LOCAL_P (x)
 	  && !SYMBOL_REF_EXTERNAL_P (x)
+	  && !SYMBOL_REF_WEAK (x)
+	  && !DECL_ONE_ONLY (SYMBOL_REF_DECL (x))
 	  && mips_use_mips16_mode_p (SYMBOL_REF_DECL (x)));
 }
 
@@ -6063,7 +6065,8 @@ mips16_local_alias (rtx func)
 	 __fn_local_* is based on the __fn_stub_* names that we've
 	 traditionally used for the non-MIPS16 stub.  */
       func_name = targetm.strip_name_encoding (XSTR (func, 0));
-      local_name = ACONCAT (("__fn_local_", func_name, NULL));
+      local_name = ACONCAT ((LOCAL_LABEL_PREFIX, "__fn_local_", func_name,
+			     NULL));
       local = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (local_name));
       SYMBOL_REF_FLAGS (local) = SYMBOL_REF_FLAGS (func) | SYMBOL_FLAG_LOCAL;
 
@@ -6190,20 +6193,15 @@ mips_output_args_xfer (int fp_code, char
    into the general registers and then jumps to the MIPS16 code.  */
 
 static void
-mips16_build_function_stub (void)
+mips16_build_function_stub (rtx symbol, const char *fnname, 
+			    rtx alias)
 {
-  const char *fnname, *alias_name, *separator;
+  const char *separator;
   char *secname, *stubname;
   tree stubdecl;
   unsigned int f;
-  rtx symbol, alias;
-
-  /* Create the name of the stub, and its unique section.  */
-  symbol = XEXP (DECL_RTL (current_function_decl), 0);
-  alias = mips16_local_alias (symbol);
 
-  fnname = targetm.strip_name_encoding (XSTR (symbol, 0));
-  alias_name = targetm.strip_name_encoding (XSTR (alias, 0));
+  /* Create the name of the unique section for the stub.  */
   secname = ACONCAT ((".mips16.fn.", fnname, NULL));
   stubname = ACONCAT (("__fn_stub_", fnname, NULL));
 
@@ -6215,6 +6213,12 @@ mips16_build_function_stub (void)
   DECL_RESULT (stubdecl) = build_decl (BUILTINS_LOCATION,
 				       RESULT_DECL, NULL_TREE, void_type_node);
 
+  /* If the original function should occur only once in the final
+     binary (e.g. it's in a COMDAT group), the same should be true of
+     the stub.  */
+  if (DECL_ONE_ONLY (current_function_decl))
+    make_decl_one_only (stubdecl, DECL_ASSEMBLER_NAME (current_function_decl));
+
   /* Output a comment.  */
   fprintf (asm_out_file, "\t# Stub function for %s (",
 	   current_function_name ());
@@ -6265,6 +6269,25 @@ mips16_build_function_stub (void)
 
   mips_end_function_definition (stubname);
 
+  switch_to_section (function_section (current_function_decl));
+}
+
+static void
+mips16_build_function_stub_and_local_alias (bool need_stub)
+{
+  const char *fnname, *alias_name;
+  rtx symbol, alias;
+
+  /* Determine the name of the alias.  */
+  symbol = XEXP (DECL_RTL (current_function_decl), 0);
+  alias = mips16_local_alias (symbol);
+  fnname = targetm.strip_name_encoding (XSTR (symbol, 0));
+  alias_name = targetm.strip_name_encoding (XSTR (alias, 0));
+
+  /* Build the stub, if necessary.  */
+  if (need_stub)
+    mips16_build_function_stub (symbol, fnname, alias);
+
   /* If the linker needs to create a dynamic symbol for the target
      function, it will associate the symbol with the stub (which,
      unlike the target function, follows the proper calling conventions).
@@ -6273,8 +6296,6 @@ mips16_build_function_stub (void)
      this symbol can also be used for indirect MIPS16 references from
      within this file.  */
   ASM_OUTPUT_DEF (asm_out_file, alias_name, fnname);
-
-  switch_to_section (function_section (current_function_decl));
 }
 
 /* The current function is a MIPS16 function that returns a value in an FPR.
@@ -6388,8 +6409,11 @@ mips16_build_call_stub (rtx retval, rtx 
       bool lazy_p;
 
       /* If this is a locally-defined and locally-binding function,
-	 avoid the stub by calling the local alias directly.  */
-      if (mips16_local_function_p (fn))
+	 avoid the stub by calling the local alias directly.
+	 Functions that return floating-point values but do not take
+	 floating-point arguments do not have a local alias, so we
+	 cannot take this short-cut in that case.  */
+      if (fp_code && mips16_local_function_p (fn))
 	{
 	  *fn_ptr = mips16_local_alias (fn);
 	  return NULL_RTX;
@@ -6445,7 +6469,7 @@ mips16_build_call_stub (rtx retval, rtx 
     {
       const char *separator;
       char *secname, *stubname;
-      tree stubid, stubdecl;
+      tree stubid, stubdecl, targetdecl;
       unsigned int f;
 
       /* If the function does not return in FPRs, the special stub
@@ -6470,6 +6494,14 @@ mips16_build_call_stub (rtx retval, rtx 
 					   RESULT_DECL, NULL_TREE,
 					   void_type_node);
 
+      targetdecl = SYMBOL_REF_DECL (fn);
+
+      /* If the called function should occur only once in the final binary
+	 (e.g. it's in a COMDAT group), the same should be true of the
+	 stub.  */
+      if (targetdecl && DECL_ONE_ONLY (targetdecl))
+	make_decl_one_only (stubdecl, DECL_ASSEMBLER_NAME (targetdecl));
+
       /* Output a comment.  */
       fprintf (asm_out_file, "\t# Stub function to call %s%s (",
 	       (fp_ret_p
@@ -10201,10 +10233,15 @@ mips_output_function_prologue (FILE *fil
 
   /* In MIPS16 mode, we may need to generate a non-MIPS16 stub to handle
      floating-point arguments.  */
-  if (TARGET_MIPS16
-      && TARGET_HARD_FLOAT_ABI
-      && crtl->args.info.fp_code != 0)
-    mips16_build_function_stub ();
+  if (TARGET_MIPS16 && TARGET_HARD_FLOAT_ABI)
+    {
+      bool fp_args = crtl->args.info.fp_code != 0;
+      rtx return_rtx = crtl->return_rtx;
+      bool fp_ret= (return_rtx 
+		    && mips_return_mode_in_fpr_p (GET_MODE (return_rtx)));
+      if (fp_args || fp_ret)
+	mips16_build_function_stub_and_local_alias (fp_args);
+    }
 
   /* Get the function name the same way that toplev.c does before calling
      assemble_start_function.  This is needed so that the name used here
Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp	(revision 190188)
+++ gcc/testsuite/gcc.target/mips/mips.exp	(working copy)
@@ -266,6 +266,7 @@ foreach option {
     synci
     relax-pic-calls
     mcount-ra-address
+    interlink-mips16
 } {
     lappend mips_option_groups $option "-m(no-|)$option"
 }
Index: gcc/testsuite/gcc.target/mips/mips16-fn-local.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mips16-fn-local.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/mips16-fn-local.c	(revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do link { target fpic } } */
+/* { dg-options "(-mips16) -fpic -minterlink-mips16" } */
+MIPS16 static double
+fn1(void) { return 0.0; }
+MIPS16 static double
+fn2(double d) { return 0.0; }
+MIPS16 void
+main(void) {
+  volatile double d;
+  d = fn1();
+  d = fn2(0.0);
+}