diff mbox

[sched-deps] Generalise usage of macro fusion to work on any two insns

Message ID 53C3AA6D.1060405@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov July 14, 2014, 10:01 a.m. UTC 
On 11/07/14 14:20, Alexander Monakov wrote:
> On Fri, 11 Jul 2014, Kyrill Tkachov wrote:
>> On 10/07/14 22:53, Maxim Kuvyrkov wrote:
>>> The patch looks good to me, but some cleanup suggestions below.
>> Thanks, here's an updated patch.
>> How's this?
> You need to remove 'if (targetm. ...) SCHED_GROUP_P (insn) = 1;' from the
> first if branch, keeping only one SCHED_GROUP_P assignment at the end of the
> function.
>
> Alexander

Thanks for the pointer, I had hurried a bit.
Here is the updated patch.

Kyrill

2014-07-14  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.

Comments

Kyrylo Tkachov July 24, 2014, 9:11 a.m. UTC | #1 
Ping.
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00958.html

Kyrill

On 14/07/14 11:01, Kyrill Tkachov wrote:
> On 11/07/14 14:20, Alexander Monakov wrote:
>> On Fri, 11 Jul 2014, Kyrill Tkachov wrote:
>>> On 10/07/14 22:53, Maxim Kuvyrkov wrote:
>>>> The patch looks good to me, but some cleanup suggestions below.
>>> Thanks, here's an updated patch.
>>> How's this?
>> You need to remove 'if (targetm. ...) SCHED_GROUP_P (insn) = 1;' from the
>> first if branch, keeping only one SCHED_GROUP_P assignment at the end of the
>> function.
>>
>> Alexander
> Thanks for the pointer, I had hurried a bit.
> Here is the updated patch.
>
> Kyrill
>
> 2014-07-14  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.
Maxim Kuvyrkov July 24, 2014, 10:10 a.m. UTC | #2 
On Jul 24, 2014, at 10:11 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

> Ping.
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00958.html
> 
> Kyrill
> 
> On 14/07/14 11:01, Kyrill Tkachov wrote:
>> On 11/07/14 14:20, Alexander Monakov wrote:
>>> On Fri, 11 Jul 2014, Kyrill Tkachov wrote:
>>>> On 10/07/14 22:53, Maxim Kuvyrkov wrote:
>>>>> The patch looks good to me, but some cleanup suggestions below.
>>>> Thanks, here's an updated patch.
>>>> How's this?
>>> You need to remove 'if (targetm. ...) SCHED_GROUP_P (insn) = 1;' from the
>>> first if branch, keeping only one SCHED_GROUP_P assignment at the end of the
>>> function.
>>> 
>>> Alexander
>> Thanks for the pointer, I had hurried a bit.
>> Here is the updated patch.
>> 

Hi Kyrill,

I have reviewed the latest version of your patch and it is perfectly fine with me.  You need to wait for an ack from the official maintainer to commit your patch.

Thank you,

--
Maxim Kuvyrkov
www.linaro.org

>> Kyrill
>> 
>> 2014-07-14  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.
> 
>
Jeff Law July 25, 2014, 9:22 p.m. UTC | #3 
On 07/24/14 03:11, Kyrill Tkachov wrote:
> Ping.
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00958.html
>
> Kyrill
>
> On 14/07/14 11:01, Kyrill Tkachov wrote:
>> On 11/07/14 14:20, Alexander Monakov wrote:
>>> On Fri, 11 Jul 2014, Kyrill Tkachov wrote:
>>>> On 10/07/14 22:53, Maxim Kuvyrkov wrote:
>>>>> The patch looks good to me, but some cleanup suggestions below.
>>>> Thanks, here's an updated patch.
>>>> How's this?
>>> You need to remove 'if (targetm. ...) SCHED_GROUP_P (insn) = 1;' from
>>> the
>>> first if branch, keeping only one SCHED_GROUP_P assignment at the end
>>> of the
>>> function.
>>>
>>> Alexander
>> Thanks for the pointer, I had hurried a bit.
>> Here is the updated patch.
>>
>> Kyrill
>>
>> 2014-07-14  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.
This is fine.  Thanks for your patience.

jeff
diff mbox

Patch

commit 643a8658f1788de2301d9d6a0457979c06afbdf9
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..ae2fff2 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2820,35 +2820,42 @@  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;
+    }
+  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 +2884,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