From patchwork Tue Aug 7 20:26:13 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sandra Loosemore X-Patchwork-Id: 175798 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id A8CC22C0097 for ; Wed, 8 Aug 2012 06:26:34 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1344975995; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=WTf88GD NKTbqgyGN78/qqWceQzY=; b=XklFHE900R7ULbd8ShuqWfpiA0ebwoDkXB38fA6 hafI+Q6s3KaT/HTuFO6shONhw09709dWdjsoTIR8TwZYbNm+tALbTxTwxh9tilM6 +kESsALbOGHI8G9k55zJb3NNTAOTZG2oAPpaj7Iu259FjvfZq6b3nkyAUyhtaYiA c10E= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=POw+gv4NIY5qRYYdigORbiylKxS7i1CUdHAzMHDFnLIn8h/qQQ5gDlyMDaahnj 8F9UvHI96JE/M7KkmtzOz97w7kDWBTCOEtbMm/RZwNintK0YeshEdRWLzJDAe5L6 LD4xl+ouM7tYW7mUnUhpiwatxSyd17J6oiYDadqf4pjOo=; Received: (qmail 6522 invoked by alias); 7 Aug 2012 20:26:31 -0000 Received: (qmail 6511 invoked by uid 22791); 7 Aug 2012 20:26:28 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN, KHOP_RCVD_UNTRUST, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, TW_FN X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 07 Aug 2012 20:26:14 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SyqM1-0000Q6-0Y from Sandra_Loosemore@mentor.com for gcc-patches@gcc.gnu.org; Tue, 07 Aug 2012 13:26:13 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Tue, 7 Aug 2012 13:26:12 -0700 Received: from [IPv6:::1] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.1.289.1; Tue, 7 Aug 2012 13:26:11 -0700 Message-ID: <502179E5.7070301@codesourcery.com> Date: Tue, 7 Aug 2012 14:26:13 -0600 From: Sandra Loosemore User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: Subject: [PATCH, MIPS] fix MIPS16 hard-float function stub bugs Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 Maciej W. Rozycki Julian Brown 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); +}