From patchwork Fri Jul 11 12:45:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 369153 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id C80181400D6 for ; Fri, 11 Jul 2014 22:45:30 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=IMQawe8wu43itXv/A cw8HGCIWJ8+ZA907NixW7sLe9UWEjVWYs4wU6XcqH4IrzKCTBeHy6g98fciSsNPG iK6MwQWzlU/Xa6wBFYBSV++co0w3PrzvK2O2A/uoannl6VnXjUQIVkcri0xf2vlI kI+MOokG4TpuuKuXxb7ohGYSqY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=ptO9CxDIKc5TIDyNvPsVfFF yQQs=; b=RbrU4bTyzYV+I3T3Gvtljg7Sd9CHmt5/ZvD5GbRdSOHeOsmLwhsMrmQ le7TFqrBDuzRilAcHC2OdK41Z7kTyQ4oERpqb30yLIuMOmpmeiPufdwdLbBQsYFt AL8x/8+aXORXcpGx2QLeyo2Bya5Z1VhB1kC0N4Uraf2eT9EBQcnY= Received: (qmail 29825 invoked by alias); 11 Jul 2014 12:45:22 -0000 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 Received: (qmail 29812 invoked by uid 89); 11 Jul 2014 12:45:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jul 2014 12:45:17 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 11 Jul 2014 13:45:14 +0100 Received: from [10.1.208.24] ([10.1.255.212]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 11 Jul 2014 13:45:11 +0100 Message-ID: <53BFDC57.9070003@arm.com> Date: Fri, 11 Jul 2014 13:45:11 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Maxim Kuvyrkov CC: Jeff Law , GCC Patches , Ramana Radhakrishnan Subject: Re: [PATCH][sched-deps] Generalise usage of macro fusion to work on any two insns References: <53AD2B65.6030406@arm.com> <53B1CB16.10206@redhat.com> <53BE4804.4010709@arm.com> <094CE8DC-1AF8-44E2-BCF6-63811F8A4835@linaro.org> In-Reply-To: <094CE8DC-1AF8-44E2-BCF6-63811F8A4835@linaro.org> X-MC-Unique: 114071113451402801 X-IsSubscribed: yes On 10/07/14 22:53, Maxim Kuvyrkov wrote: > On Jul 10, 2014, at 8:00 PM, Kyrill Tkachov wrote: > >> On 30/06/14 21:39, Jeff Law wrote: >>> On 06/27/14 02:29, Kyrill Tkachov wrote: >>>> Hi all, >>>> >>>> This patch generalises the TARGET_MACRO_FUSION_PAIR_P hook usage to work >>>> on more than just >>>> compares and conditional branches for which it was initially designed >>>> for (for x86). >>>> >>>> There are some instructions in arm and aarch64 that can be fused >>>> together when they're back to back in the instruction stream and I'd >>>> like to use this hook to keep them together. >>>> >>>> I'll post an implementation of TARGET_MACRO_FUSION_PAIR_P for arm and >>>> aarch64 shortly... >>>> >>>> Bootstrapped and tested on x86, aarch64-none-linux-gnu and >>>> arm-none-linux-gnueabihf. >>>> >>>> Ok for trunk? > The patch looks good to me, but some cleanup suggestions below. Thanks, here's an updated patch. How's this? 2014-07-11 Ramana Radhakrishnan Kyrylo Tkachov * sched-deps.c (try_group_insn): Generalise macro fusion hook usage to any two insns. Update comment. Rename to sched_macro_fuse_insns. (sched_analyze_insn): Update use of try_group_insn to sched_macro_fuse_insns. * config/i386/i386.c (ix86_macro_fusion_pair_p): Reject 2nd arguments that are not conditional jumps. > >> commit e36b8977738dbe3f63445199710ca627ab37e243 >> Author: Kyrylo Tkachov >> Date: Fri Jun 13 11:41:41 2014 +0100 >> >> [sched-deps] Generalise macro fusion hook usage >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 8046c67..7dd2ce5 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -25817,6 +25817,9 @@ ix86_macro_fusion_pair_p (rtx condgen, rtx condjmp) >> rtx compare_set = NULL_RTX, test_if, cond; >> rtx alu_set = NULL_RTX, addr = NULL_RTX; >> >> + if (!any_condjump_p (condjmp)) >> + return false; >> + >> if (get_attr_type (condgen) != TYPE_TEST >> && get_attr_type (condgen) != TYPE_ICMP >> && get_attr_type (condgen) != TYPE_INCDEC >> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c >> index 7cafc8b..c01a8a6 100644 >> --- a/gcc/sched-deps.c >> +++ b/gcc/sched-deps.c >> @@ -2820,35 +2820,48 @@ sched_analyze_2 (struct deps_desc *deps, rtx x, rtx insn) >> sched_deps_info->finish_rhs (); >> } >> >> -/* Try to group comparison and the following conditional jump INSN if >> - they're already adjacent. This is to prevent scheduler from scheduling >> - them apart. */ >> +/* Try to group two fuseable insns together to prevent scheduler >> + from scheduling them apart. */ >> >> static void >> try_group_insn (rtx insn) > Please rename try_group_insn to sched_macro_fuse_insns. The call is predicated to try_group_insn is predicated on targetm.sched.macro_fusion_p, so this code will not be used for any other kinds of fusion -- might as well just state that in the name,. > >> { >> - unsigned int condreg1, condreg2; >> - rtx cc_reg_1; >> rtx prev; >> >> - if (!any_condjump_p (insn)) >> + if (!targetm.sched.macro_fusion_p ()) >> return; > This is a no-op since there is a check on the upper level. Please remove. > >> >> - targetm.fixed_condition_code_regs (&condreg1, &condreg2); >> - cc_reg_1 = gen_rtx_REG (CCmode, condreg1); >> - prev = prev_nonnote_nondebug_insn (insn); >> - if (!reg_referenced_p (cc_reg_1, PATTERN (insn)) >> - || !prev >> - || !modified_in_p (cc_reg_1, prev)) >> - return; >> + if (any_condjump_p (insn)) >> + { >> + unsigned int condreg1, condreg2; >> + rtx cc_reg_1; >> + targetm.fixed_condition_code_regs (&condreg1, &condreg2); >> + cc_reg_1 = gen_rtx_REG (CCmode, condreg1); >> + prev = prev_nonnote_nondebug_insn (insn); >> + if (!reg_referenced_p (cc_reg_1, PATTERN (insn)) >> + || !prev >> + || !modified_in_p (cc_reg_1, prev)) >> + return; >> >> - /* Different microarchitectures support macro fusions for different >> - combinations of insn pairs. */ >> - if (!targetm.sched.macro_fusion_pair_p >> - || !targetm.sched.macro_fusion_pair_p (prev, insn)) >> - return; >> + if (targetm.sched.macro_fusion_pair_p (prev, insn)) >> + SCHED_GROUP_P (insn) = 1; >> + } >> + else >> + { >> + rtx insn_set = single_set (insn); >> + >> + prev = prev_nonnote_nondebug_insn (insn); >> + if (prev >> + && insn_set >> + && single_set (prev) >> + && modified_in_p (SET_DEST (insn_set), prev) > Invert the check (as done in the upper if-clause) and cut it here. Then you can use a single unified > > if (targetm.sched.macro_fusion_pair_p (prev, insn)) > SCHED_GROUP_P (insn) = 1; > > as the final statement of the function. > > Thank you, > > -- > Maxim Kuvyrkov > www.linaro.org > > commit cb0584229d9247df805df35dc4c5bffbb839d59f Author: Kyrylo Tkachov Date: Fri Jun 13 11:41:41 2014 +0100 [sched-deps] Generalise macro fusion hook usage diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 1b5cbeb..6951ddd 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -25820,6 +25820,9 @@ ix86_macro_fusion_pair_p (rtx condgen, rtx condjmp) rtx compare_set = NULL_RTX, test_if, cond; rtx alu_set = NULL_RTX, addr = NULL_RTX; + if (!any_condjump_p (condjmp)) + return false; + if (get_attr_type (condgen) != TYPE_TEST && get_attr_type (condgen) != TYPE_ICMP && get_attr_type (condgen) != TYPE_INCDEC diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c index 7cafc8b..2d9c834 100644 --- a/gcc/sched-deps.c +++ b/gcc/sched-deps.c @@ -2820,35 +2820,45 @@ sched_analyze_2 (struct deps_desc *deps, rtx x, rtx insn) sched_deps_info->finish_rhs (); } -/* Try to group comparison and the following conditional jump INSN if - they're already adjacent. This is to prevent scheduler from scheduling - them apart. */ +/* Try to group two fuseable insns together to prevent scheduler + from scheduling them apart. */ static void -try_group_insn (rtx insn) +sched_macro_fuse_insns (rtx insn) { - unsigned int condreg1, condreg2; - rtx cc_reg_1; rtx prev; - if (!any_condjump_p (insn)) - return; + if (any_condjump_p (insn)) + { + unsigned int condreg1, condreg2; + rtx cc_reg_1; + targetm.fixed_condition_code_regs (&condreg1, &condreg2); + cc_reg_1 = gen_rtx_REG (CCmode, condreg1); + prev = prev_nonnote_nondebug_insn (insn); + if (!reg_referenced_p (cc_reg_1, PATTERN (insn)) + || !prev + || !modified_in_p (cc_reg_1, prev)) + return; + + if (targetm.sched.macro_fusion_pair_p (prev, insn)) + SCHED_GROUP_P (insn) = 1; + } + else + { + rtx insn_set = single_set (insn); - targetm.fixed_condition_code_regs (&condreg1, &condreg2); - cc_reg_1 = gen_rtx_REG (CCmode, condreg1); - prev = prev_nonnote_nondebug_insn (insn); - if (!reg_referenced_p (cc_reg_1, PATTERN (insn)) - || !prev - || !modified_in_p (cc_reg_1, prev)) - return; + prev = prev_nonnote_nondebug_insn (insn); + if (!prev + || !insn_set + || !single_set (prev) + || !modified_in_p (SET_DEST (insn_set), prev)) + return; - /* Different microarchitectures support macro fusions for different - combinations of insn pairs. */ - if (!targetm.sched.macro_fusion_pair_p - || !targetm.sched.macro_fusion_pair_p (prev, insn)) - return; + } + + if (targetm.sched.macro_fusion_pair_p (prev, insn)) + SCHED_GROUP_P (insn) = 1; - SCHED_GROUP_P (insn) = 1; } /* Analyze an INSN with pattern X to find all dependencies. */ @@ -2877,7 +2887,7 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, rtx insn) /* Group compare and branch insns for macro-fusion. */ if (targetm.sched.macro_fusion_p && targetm.sched.macro_fusion_p ()) - try_group_insn (insn); + sched_macro_fuse_insns (insn); if (may_trap_p (x)) /* Avoid moving trapping instructions across function calls that might