From patchwork Sun Dec 23 17:02:08 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 207989 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 D6F6A2C0040 for ; Mon, 24 Dec 2012 04:02:25 +1100 (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=1356886947; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: From:To:Mail-Followup-To:Subject:Date:Message-ID:User-Agent: MIME-Version:Content-Type:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=mQQQpOjV0ONo2wqG8c/Of8fo0gg=; b=qpMn0IOoGqfcGTi Apkr+TDasLd8wvwhlT2n5OddUDJAT6bVcBaxUMhkJBnNUf+uTwZxA9+u425huAuw +XNXCDDJE6JvqDL4Bp5d1ZexBOLA6HvK7CGyVwys0A5UAcWneXtOBsjuOgtn+CMd d61jeXF06O3F4euy5e4Swd/bZyAM= 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:X-Received:Received:From:To:Mail-Followup-To:Subject:Date:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=fWfBrLTXdI7uqT8rvwQSzdv5UleS2b+S00thqgqsRYwjopOCD4cVDznMzChxqj x5qr3zxLGfBDGQA4plcJGxHPrLQ3gVNmW7NzqlfZboExQd3mNFCjfA6GiE4pPUR/ fALWVZzl6OxjlBRU9BpwN1CD8gKvlkfcNGbyv0P0ZZORA=; Received: (qmail 7422 invoked by alias); 23 Dec 2012 17:02:21 -0000 Received: (qmail 7410 invoked by uid 22791); 23 Dec 2012 17:02:19 -0000 X-SWARE-Spam-Status: No, hits=-3.2 required=5.0 tests=AWL, BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, KHOP_RCVD_TRUST, NML_ADSP_CUSTOM_MED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, TW_FN X-Spam-Check-By: sourceware.org Received: from mail-we0-f172.google.com (HELO mail-we0-f172.google.com) (74.125.82.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 23 Dec 2012 17:02:12 +0000 Received: by mail-we0-f172.google.com with SMTP id r3so3049313wey.3 for ; Sun, 23 Dec 2012 09:02:11 -0800 (PST) X-Received: by 10.180.108.236 with SMTP id hn12mr31556089wib.6.1356282131228; Sun, 23 Dec 2012 09:02:11 -0800 (PST) Received: from localhost ([2.28.234.219]) by mx.google.com with ESMTPS id p3sm39336883wic.8.2012.12.23.09.02.09 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 23 Dec 2012 09:02:10 -0800 (PST) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Subject: RFA: Fix reuse of "target" variable in builtins.c Date: Sun, 23 Dec 2012 17:02:08 +0000 Message-ID: <878v8ovh0v.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 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 Some of the maths builtins try to expand via an optab and fall back to an expand_call. The optabs path tends to clobber "target", so the original target is lost by the time we call expand_call. E.g.: /* Compute into TARGET. Set TARGET to wherever the result comes back. */ target = expand_binop (mode, builtin_optab, op0, op1, target, 0, OPTAB_DIRECT); /* If we were unable to expand via the builtin, stop the sequence (without outputting the insns) and call to the library function with the stabilized argument list. */ if (target == 0) { end_sequence (); return expand_call (exp, target, target == const0_rtx); } where the expand_call seems to be trying to use the original call target (as it should IMO) but actually always uses null. This patch tries to fix the cases I could see. Tested on x86_64-linux-gnu and mips64-linux-gnu. OK to install? Richard gcc/ * builtins.c (expand_builtin_mathfn, expand_builtin_mathfn_2) (expand_builtin_mathfn_ternary, expand_builtin_mathfn_3) (expand_builtin_int_roundingfn_2): Keep the original target around for the fallback case. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c 2012-12-23 16:56:30.218846420 +0000 +++ gcc/builtins.c 2012-12-23 16:57:47.849415792 +0000 @@ -2031,7 +2031,7 @@ expand_builtin_mathfn (tree exp, rtx tar if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing && (!errno_set || !optimize_insn_for_size_p ())) { - target = gen_reg_rtx (mode); + rtx result = gen_reg_rtx (mode); /* Wrap the computation of the argument in a SAVE_EXPR, as we may need to expand the argument again. This way, we will not perform @@ -2042,20 +2042,20 @@ expand_builtin_mathfn (tree exp, rtx tar start_sequence (); - /* Compute into TARGET. - Set TARGET to wherever the result comes back. */ - target = expand_unop (mode, builtin_optab, op0, target, 0); + /* Compute into RESULT. + Set RESULT to wherever the result comes back. */ + result = expand_unop (mode, builtin_optab, op0, result, 0); - if (target != 0) + if (result != 0) { if (errno_set) - expand_errno_check (exp, target); + expand_errno_check (exp, result); /* Output the entire sequence. */ insns = get_insns (); end_sequence (); emit_insn (insns); - return target; + return result; } /* If we were unable to expand via the builtin, stop the sequence @@ -2078,7 +2078,7 @@ expand_builtin_mathfn (tree exp, rtx tar expand_builtin_mathfn_2 (tree exp, rtx target, rtx subtarget) { optab builtin_optab; - rtx op0, op1, insns; + rtx op0, op1, insns, result; int op1_type = REAL_TYPE; tree fndecl = get_callee_fndecl (exp); tree arg0, arg1; @@ -2134,7 +2134,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing) return NULL_RTX; - target = gen_reg_rtx (mode); + result = gen_reg_rtx (mode); if (! flag_errno_math || ! HONOR_NANS (mode)) errno_set = false; @@ -2151,29 +2151,29 @@ expand_builtin_mathfn_2 (tree exp, rtx t start_sequence (); - /* Compute into TARGET. - Set TARGET to wherever the result comes back. */ - target = expand_binop (mode, builtin_optab, op0, op1, - target, 0, OPTAB_DIRECT); + /* Compute into RESULT. + Set RESULT to wherever the result comes back. */ + result = expand_binop (mode, builtin_optab, op0, op1, + result, 0, OPTAB_DIRECT); /* If we were unable to expand via the builtin, stop the sequence (without outputting the insns) and call to the library function with the stabilized argument list. */ - if (target == 0) + if (result == 0) { end_sequence (); return expand_call (exp, target, target == const0_rtx); } if (errno_set) - expand_errno_check (exp, target); + expand_errno_check (exp, result); /* Output the entire sequence. */ insns = get_insns (); end_sequence (); emit_insn (insns); - return target; + return result; } /* Expand a call to the builtin trinary math functions (fma). @@ -2187,7 +2187,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t expand_builtin_mathfn_ternary (tree exp, rtx target, rtx subtarget) { optab builtin_optab; - rtx op0, op1, op2, insns; + rtx op0, op1, op2, insns, result; tree fndecl = get_callee_fndecl (exp); tree arg0, arg1, arg2; enum machine_mode mode; @@ -2214,7 +2214,7 @@ expand_builtin_mathfn_ternary (tree exp, if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing) return NULL_RTX; - target = gen_reg_rtx (mode); + result = gen_reg_rtx (mode); /* Always stabilize the argument list. */ CALL_EXPR_ARG (exp, 0) = arg0 = builtin_save_expr (arg0); @@ -2227,15 +2227,15 @@ expand_builtin_mathfn_ternary (tree exp, start_sequence (); - /* Compute into TARGET. - Set TARGET to wherever the result comes back. */ - target = expand_ternary_op (mode, builtin_optab, op0, op1, op2, - target, 0); + /* Compute into RESULT. + Set RESULT to wherever the result comes back. */ + result = expand_ternary_op (mode, builtin_optab, op0, op1, op2, + result, 0); /* If we were unable to expand via the builtin, stop the sequence (without outputting the insns) and call to the library function with the stabilized argument list. */ - if (target == 0) + if (result == 0) { end_sequence (); return expand_call (exp, target, target == const0_rtx); @@ -2246,7 +2246,7 @@ expand_builtin_mathfn_ternary (tree exp, end_sequence (); emit_insn (insns); - return target; + return result; } /* Expand a call to the builtin sin and cos math functions. @@ -2298,7 +2298,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t /* Before working hard, check whether the instruction is available. */ if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing) { - target = gen_reg_rtx (mode); + rtx result = gen_reg_rtx (mode); /* Wrap the computation of the argument in a SAVE_EXPR, as we may need to expand the argument again. This way, we will not perform @@ -2309,37 +2309,35 @@ expand_builtin_mathfn_3 (tree exp, rtx t start_sequence (); - /* Compute into TARGET. - Set TARGET to wherever the result comes back. */ + /* Compute into RESULT. + Set RESULT to wherever the result comes back. */ if (builtin_optab == sincos_optab) { - int result; + int ok; switch (DECL_FUNCTION_CODE (fndecl)) { CASE_FLT_FN (BUILT_IN_SIN): - result = expand_twoval_unop (builtin_optab, op0, 0, target, 0); + ok = expand_twoval_unop (builtin_optab, op0, 0, result, 0); break; CASE_FLT_FN (BUILT_IN_COS): - result = expand_twoval_unop (builtin_optab, op0, target, 0, 0); + ok = expand_twoval_unop (builtin_optab, op0, result, 0, 0); break; default: gcc_unreachable (); } - gcc_assert (result); + gcc_assert (ok); } else - { - target = expand_unop (mode, builtin_optab, op0, target, 0); - } + result = expand_unop (mode, builtin_optab, op0, result, 0); - if (target != 0) + if (result != 0) { /* Output the entire sequence. */ insns = get_insns (); end_sequence (); emit_insn (insns); - return target; + return result; } /* If we were unable to expand via the builtin, stop the sequence @@ -2348,9 +2346,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t end_sequence (); } - target = expand_call (exp, target, target == const0_rtx); - - return target; + return expand_call (exp, target, target == const0_rtx); } /* Given an interclass math builtin decl FNDECL and it's argument ARG @@ -2820,7 +2816,7 @@ expand_builtin_int_roundingfn_2 (tree ex /* There's no easy way to detect the case we need to set EDOM. */ if (!flag_errno_math) { - target = gen_reg_rtx (mode); + rtx result = gen_reg_rtx (mode); /* Wrap the computation of the argument in a SAVE_EXPR, as we may need to expand the argument again. This way, we will not perform @@ -2831,13 +2827,13 @@ expand_builtin_int_roundingfn_2 (tree ex start_sequence (); - if (expand_sfix_optab (target, op0, builtin_optab)) + if (expand_sfix_optab (result, op0, builtin_optab)) { /* Output the entire sequence. */ insns = get_insns (); end_sequence (); emit_insn (insns); - return target; + return result; } /* If we were unable to expand via the builtin, stop the sequence @@ -2865,9 +2861,7 @@ expand_builtin_int_roundingfn_2 (tree ex return convert_to_mode (mode, target, 0); } - target = expand_call (exp, target, target == const0_rtx); - - return target; + return expand_call (exp, target, target == const0_rtx); } /* Expand a call to the powi built-in mathematical function. Return NULL_RTX if