This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][sched-deps] Generalise usage of macro fusion to work on any two insns
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- Cc: Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>
- Date: Fri, 11 Jul 2014 13:45:11 +0100
- Subject: Re: [PATCH][sched-deps] Generalise usage of macro fusion to work on any two insns
- Authentication-results: sourceware.org; auth=none
- References: <53AD2B65 dot 6030406 at arm dot com> <53B1CB16 dot 10206 at redhat dot com> <53BE4804 dot 4010709 at arm dot com> <094CE8DC-1AF8-44E2-BCF6-63811F8A4835 at linaro dot org>
On 10/07/14 22:53, Maxim Kuvyrkov wrote:
On Jul 10, 2014, at 8:00 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> 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 <ramana.radhakrishnan@arm.com>
Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* 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 <kyrylo.tkachov@arm.com>
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 <kyrylo.tkachov@arm.com>
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