[PATCH] Move the check for any_condjump_p from sched-deps to target macros

[PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

Hi,

Please find attached the patch that moves the check for CC usage in
any_condjump_p from sched-deps to target macros.

Currently the check is used only by i386 and AArch64.
The general condition checks for the fusion candidates to use/modify CC1
register. However, the fusion of ALU and Branch instruction in AArch64 looks
for any register and hence does not satisfy the condition. 

Bootstrapped and Regression tested on AArch64 and X86_64.

Please review the patch and let us know if its okay?

Thanks,
Naveen

2017-03-27  Naveen H.S  <Naveen.Hurugalawadi@cavium.com>

	* config/aarch64/aarch64.c (aarch_macro_fusion_pair_p): Push the
	check for CC usage into AARCH64_FUSE_CMP_BRANCH.
	* config/i386/i386.c (ix86_macro_fusion_pair_p): Push the check for
	CC usage from generic code.
	* sched-deps.c (sched_macro_fuse_insns): Move the condition for
	any_condjump_p into the target macros.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fusion-anycond-jump.patch --]
[-- Type: text/x-patch; name="fusion-anycond-jump.patch", Size: 2701 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4f769a4..ec0a3ec 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13972,6 +13972,15 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+      if (!reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  || !prev
+	  || !modified_in_p (cc_reg_1, prev))
+	return false;
+
       /* FIXME: this misses some which is considered simple arthematic
          instructions for ThunderX.  Simple shifts are missed here.  */
       if (prev_type == TYPE_ALUS_SREG
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index bb0debf..3dcbe37 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29490,6 +29490,15 @@ ix86_macro_fusion_pair_p (rtx_insn *condgen, rtx_insn *condjmp)
   if (!any_condjump_p (condjmp))
     return false;
 
+  unsigned int condreg1, condreg2;
+  rtx cc_reg_1;
+  ix86_fixed_condition_code_regs (&condreg1, &condreg2);
+  cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+  if (!reg_referenced_p (cc_reg_1, PATTERN (condjmp))
+      || !condgen
+      || !modified_in_p (cc_reg_1, condgen))
+    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 b2393bf..b15a865 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2835,33 +2835,16 @@ sched_macro_fuse_insns (rtx_insn *insn)
 {
   rtx_insn *prev;
 
-  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);
-
-      prev = prev_nonnote_nondebug_insn (insn);
-      if (!prev
-          || !insn_set
-          || !single_set (prev))
-        return;
+  rtx insn_set = single_set (insn);
 
-    }
+  prev = prev_nonnote_nondebug_insn (insn);
+  if (!prev
+      || !insn_set
+      || !single_set (prev))
+    return;
 
   if (targetm.sched.macro_fusion_pair_p (prev, insn))
     SCHED_GROUP_P (insn) = 1;
-
 }
 
 /* Get the implicit reg pending clobbers for INSN and save them in TEMP.  */

[PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

Hi,  

Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same.  

https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01368.html

Thanks,
Naveen


    

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Wilco Dijkstra]

Hi Naveen, 

> https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01368.html

This looks good to me - I have just one comment:

--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13972,6 +13972,15 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+      if (!reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  || !prev
+	  || !modified_in_p (cc_reg_1, prev))
+	return false;
+

The return false seems incorrect - it means a core can either have FUSE_CMP_BRANCH or
FUSE_ALU_BRANCH but not both.

The way aarch_macro_fusion_pair_p works is to return true if fusion is possible but fallthrough
if not so other, less likely, fusion candidates can still be tried.

Wilco

        

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

Hi Wilco,

Thanks for reviewing the patch.

>> The return false seems incorrect - it means a core can either have
>> FUSE_CMP_BRANCH or FUSE_ALU_BRANCH but not both.

Thanks for pointing out about the confusion.
Modified the code as required.

Bootstrapped and Regression tested on AArch64 and X86_64.
Please review the patch and let us know if its okay?

Thanks,
Naveen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fusion-anycond-jump-1.patch --]
[-- Type: text/x-diff; name="fusion-anycond-jump-1.patch", Size: 2700 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 2e385c4..1a63ad0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13973,6 +13974,15 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+      if (!reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  || !prev
+	  || !modified_in_p (cc_reg_1, prev))
+	return true;
+
       /* FIXME: this misses some which is considered simple arthematic
          instructions for ThunderX.  Simple shifts are missed here.  */
       if (prev_type == TYPE_ALUS_SREG
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b2fa1b..af14c90 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29483,6 +29483,15 @@ ix86_macro_fusion_pair_p (rtx_insn *condgen, rtx_insn *condjmp)
   if (!any_condjump_p (condjmp))
     return false;
 
+  unsigned int condreg1, condreg2;
+  rtx cc_reg_1;
+  ix86_fixed_condition_code_regs (&condreg1, &condreg2);
+  cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+  if (!reg_referenced_p (cc_reg_1, PATTERN (condjmp))
+      || !condgen
+      || !modified_in_p (cc_reg_1, condgen))
+    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 b2393bf..b15a865 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2835,33 +2835,16 @@ sched_macro_fuse_insns (rtx_insn *insn)
 {
   rtx_insn *prev;
 
-  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);
-
-      prev = prev_nonnote_nondebug_insn (insn);
-      if (!prev
-          || !insn_set
-          || !single_set (prev))
-        return;
+  rtx insn_set = single_set (insn);
 
-    }
+  prev = prev_nonnote_nondebug_insn (insn);
+  if (!prev
+      || !insn_set
+      || !single_set (prev))
+    return;
 
   if (targetm.sched.macro_fusion_pair_p (prev, insn))
     SCHED_GROUP_P (insn) = 1;
-
 }
 
 /* Get the implicit reg pending clobbers for INSN and save them in TEMP.  */

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

Hi Wilco,

>> I suggest you check the logic and follow the existing patterns in
>> aarch_macro_fusion_pair_p.

Done.

Bootstrapped and Regression tested on AArch64 and X86_64.
Please review the patch and let us know if its okay?

Thanks,
Naveen
     

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fusion-anycond-jump-2.patch --]
[-- Type: text/x-patch; name="fusion-anycond-jump-2.patch", Size: 3180 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1e58e9d..9f838f5 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -14022,13 +14022,23 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
-      /* FIXME: this misses some which is considered simple arthematic
-         instructions for ThunderX.  Simple shifts are missed here.  */
-      if (prev_type == TYPE_ALUS_SREG
-          || prev_type == TYPE_ALUS_IMM
-          || prev_type == TYPE_LOGICS_REG
-          || prev_type == TYPE_LOGICS_IMM)
-        return true;
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+
+      if (reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  && prev
+	  && modified_in_p (cc_reg_1, prev))
+	{
+	  /* FIXME: this misses some which is considered simple arthematic
+	     instructions for ThunderX.  Simple shifts are missed here.  */
+	  if (prev_type == TYPE_ALUS_SREG
+	      || prev_type == TYPE_ALUS_IMM
+	      || prev_type == TYPE_LOGICS_REG
+	      || prev_type == TYPE_LOGICS_IMM)
+	    return true;
+	}
     }
 
   return false;
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d985657..3352189 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29610,6 +29610,15 @@ ix86_macro_fusion_pair_p (rtx_insn *condgen, rtx_insn *condjmp)
   if (!any_condjump_p (condjmp))
     return false;
 
+  unsigned int condreg1, condreg2;
+  rtx cc_reg_1;
+  ix86_fixed_condition_code_regs (&condreg1, &condreg2);
+  cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+  if (!reg_referenced_p (cc_reg_1, PATTERN (condjmp))
+      || !condgen
+      || !modified_in_p (cc_reg_1, condgen))
+    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 b2393bf..b15a865 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2835,33 +2835,16 @@ sched_macro_fuse_insns (rtx_insn *insn)
 {
   rtx_insn *prev;
 
-  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);
-
-      prev = prev_nonnote_nondebug_insn (insn);
-      if (!prev
-          || !insn_set
-          || !single_set (prev))
-        return;
+  rtx insn_set = single_set (insn);
 
-    }
+  prev = prev_nonnote_nondebug_insn (insn);
+  if (!prev
+      || !insn_set
+      || !single_set (prev))
+    return;
 
   if (targetm.sched.macro_fusion_pair_p (prev, insn))
     SCHED_GROUP_P (insn) = 1;
-
 }
 
 /* Get the implicit reg pending clobbers for INSN and save them in TEMP.  */

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Jeff Law]

On 04/26/2017 06:50 AM, Hurugalawadi, Naveen wrote:
> Hi Wilco,
> 
> Thanks for reviewing the patch.
> 
>>> The return false seems incorrect - it means a core can either have
>>> FUSE_CMP_BRANCH or FUSE_ALU_BRANCH but not both.
> Thanks for pointing out about the confusion.
> Modified the code as required.
> 
> Bootstrapped and Regression tested on AArch64 and X86_64.
> Please review the patch and let us know if its okay?
> 
> Thanks,
> Naveen
> 
> 
> fusion-anycond-jump-1.patch
> 
> 
> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
> index b2393bf..b15a865 100644
> --- a/gcc/sched-deps.c
> +++ b/gcc/sched-deps.c
> @@ -2835,33 +2835,16 @@ sched_macro_fuse_insns (rtx_insn *insn)
>   {
>     rtx_insn *prev;
>   
> -  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);
> -
> -      prev = prev_nonnote_nondebug_insn (insn);
> -      if (!prev
> -          || !insn_set
> -          || !single_set (prev))
> -        return;
> +  rtx insn_set = single_set (insn);
>   
> -    }
> +  prev = prev_nonnote_nondebug_insn (insn);
> +  if (!prev
> +      || !insn_set
> +      || !single_set (prev))
> +    return;
>   
>     if (targetm.sched.macro_fusion_pair_p (prev, insn))
>       SCHED_GROUP_P (insn) = 1;
> -
>   }
Doesn't this avoid calling the target hook in cases where it used to 
call it before?

Consider a conditional jump inside a parallel that is not a single set.

Previously that could call the target hook and potentially set 
SCHED_GROUP_P.  But after your change I think it would return without 
ever querying the backend.

jeff

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

Hi,

>> Doesn't this avoid calling the target hook in cases where it used to 
>> call it before?

Yes. Thanks for pointing it out.

>> Consider a conditional jump inside a parallel that is not a single set.

Please find attached the modified patch that handles the case mentioned.
Please review the patch and let us know if its okay?

Bootstrapped and Regression tested on AArch64 and X86_64.
Please review the patch and let us know if its okay?

Thanks,
Naveen
    

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fusion-anycond-jump-4.patch --]
[-- Type: text/x-patch; name="fusion-anycond-jump-4.patch", Size: 3453 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 2e385c4..b38b8b7 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13973,13 +13973,23 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
-      /* FIXME: this misses some which is considered simple arthematic
-         instructions for ThunderX.  Simple shifts are missed here.  */
-      if (prev_type == TYPE_ALUS_SREG
-          || prev_type == TYPE_ALUS_IMM
-          || prev_type == TYPE_LOGICS_REG
-          || prev_type == TYPE_LOGICS_IMM)
-        return true;
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+
+      if (reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  && prev
+	  && modified_in_p (cc_reg_1, prev))
+	{
+	  /* FIXME: this misses some which is considered simple arthematic
+	     instructions for ThunderX.  Simple shifts are missed here.  */
+	  if (prev_type == TYPE_ALUS_SREG
+	      || prev_type == TYPE_ALUS_IMM
+	      || prev_type == TYPE_LOGICS_REG
+	      || prev_type == TYPE_LOGICS_IMM)
+	    return true;
+	}
     }
 
   return false;
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b2fa1b..af14c90 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29483,6 +29483,15 @@ ix86_macro_fusion_pair_p (rtx_insn *condgen, rtx_insn *condjmp)
   if (!any_condjump_p (condjmp))
     return false;
 
+  unsigned int condreg1, condreg2;
+  rtx cc_reg_1;
+  ix86_fixed_condition_code_regs (&condreg1, &condreg2);
+  cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+  if (!reg_referenced_p (cc_reg_1, PATTERN (condjmp))
+      || !condgen
+      || !modified_in_p (cc_reg_1, condgen))
+    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 b2393bf..4c459e6 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2834,34 +2834,30 @@ static void
 sched_macro_fuse_insns (rtx_insn *insn)
 {
   rtx_insn *prev;
-
+  prev = prev_nonnote_nondebug_insn (insn);
+  if (!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;
+      if (reg_referenced_p (cc_reg_1, PATTERN (insn))
+	  && modified_in_p (cc_reg_1, prev))
+	{
+	  if (targetm.sched.macro_fusion_pair_p (prev, insn))
+	    SCHED_GROUP_P (insn) = 1;
+	  return;
+	}
     }
-  else
-    {
-      rtx insn_set = single_set (insn);
-
-      prev = prev_nonnote_nondebug_insn (insn);
-      if (!prev
-          || !insn_set
-          || !single_set (prev))
-        return;
 
+  if (single_set (insn) && single_set (prev))
+    {
+      if (targetm.sched.macro_fusion_pair_p (prev, insn))
+	SCHED_GROUP_P (insn) = 1;
     }
-
-  if (targetm.sched.macro_fusion_pair_p (prev, insn))
-    SCHED_GROUP_P (insn) = 1;
-
 }
 
 /* Get the implicit reg pending clobbers for INSN and save them in TEMP.  */

Re: [PING 2][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

Hi, 

Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same. 

https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00839.html

Thanks,
Naveen
        

Re: [PING 2][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Wilco Dijkstra]

Hurugalawadi, Naveen <Naveen.Hurugalawadi@cavium.com> wrote:
>
> Please consider this as a personal reminder to review the patch
> at following link and let me know your comments on the same. 
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00839.htmll

That looks good to me now.

Wilco

Re: [PING 3][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

Hi Wilco,

>> That looks good to me now.

Thanks for the review and your okay for the patch.

Please consider this as a personal reminder to review the patch
at following link and let me know if its okay to commit?

https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00839.html

Thanks,
Naveen

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Jeff Law]

On 05/10/2017 10:46 PM, Hurugalawadi, Naveen wrote:
> Hi,
> 
>>> Doesn't this avoid calling the target hook in cases where it used to 
>>> call it before?
> Yes. Thanks for pointing it out.
> 
>>> Consider a conditional jump inside a parallel that is not a single set.
> Please find attached the modified patch that handles the case mentioned.
> Please review the patch and let us know if its okay?
> 
> Bootstrapped and Regression tested on AArch64 and X86_64.
> Please review the patch and let us know if its okay?
SOrry it's taken so long to come back to this.

The code is a bit convoluted, but I think you've preserved the existing
logic.    You need a ChangeLog entry, but I think that's it.  Can you
please repost with a ChangeLog entry for final approval?

Thanks,

JEff

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Hurugalawadi, Naveen]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

Hi Jeff,

Thanks for the review and your approval for final patch.
Sorry, It was a long weekend and hence could not revert to your
comments earlier.

>> You need a ChangeLog entry, but I think that's it.  Can you
>> please repost with a ChangeLog entry for final approval?

Please find the final patch and ChangeLog entry updated as required.
Please review the same and let me know if its okay to commit?

Thanks,
Naveen

2017-06-27  Naveen H.S  <Naveen.Hurugalawadi@cavium.com>

	* config/aarch64/aarch64.c (aarch_macro_fusion_pair_p): Push the
	check for CC usage into AARCH64_FUSE_CMP_BRANCH.
	* config/i386/i386.c (ix86_macro_fusion_pair_p): Push the check for
	CC usage from generic code to here.
	* sched-deps.c (sched_macro_fuse_insns): Move the condition for
	CC usage into the target macros.    

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fusion-anycond-jump-4.patch --]
[-- Type: text/x-diff; name="fusion-anycond-jump-4.patch", Size: 3453 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 2e385c4..b38b8b7 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13973,13 +13973,23 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
     {
       enum attr_type prev_type = get_attr_type (prev);
 
-      /* FIXME: this misses some which is considered simple arthematic
-         instructions for ThunderX.  Simple shifts are missed here.  */
-      if (prev_type == TYPE_ALUS_SREG
-          || prev_type == TYPE_ALUS_IMM
-          || prev_type == TYPE_LOGICS_REG
-          || prev_type == TYPE_LOGICS_IMM)
-        return true;
+      unsigned int condreg1, condreg2;
+      rtx cc_reg_1;
+      aarch64_fixed_condition_code_regs (&condreg1, &condreg2);
+      cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+
+      if (reg_referenced_p (cc_reg_1, PATTERN (curr))
+	  && prev
+	  && modified_in_p (cc_reg_1, prev))
+	{
+	  /* FIXME: this misses some which is considered simple arthematic
+	     instructions for ThunderX.  Simple shifts are missed here.  */
+	  if (prev_type == TYPE_ALUS_SREG
+	      || prev_type == TYPE_ALUS_IMM
+	      || prev_type == TYPE_LOGICS_REG
+	      || prev_type == TYPE_LOGICS_IMM)
+	    return true;
+	}
     }
 
   return false;
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b2fa1b..af14c90 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29483,6 +29483,15 @@ ix86_macro_fusion_pair_p (rtx_insn *condgen, rtx_insn *condjmp)
   if (!any_condjump_p (condjmp))
     return false;
 
+  unsigned int condreg1, condreg2;
+  rtx cc_reg_1;
+  ix86_fixed_condition_code_regs (&condreg1, &condreg2);
+  cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+  if (!reg_referenced_p (cc_reg_1, PATTERN (condjmp))
+      || !condgen
+      || !modified_in_p (cc_reg_1, condgen))
+    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 b2393bf..4c459e6 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2834,34 +2834,30 @@ static void
 sched_macro_fuse_insns (rtx_insn *insn)
 {
   rtx_insn *prev;
-
+  prev = prev_nonnote_nondebug_insn (insn);
+  if (!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;
+      if (reg_referenced_p (cc_reg_1, PATTERN (insn))
+	  && modified_in_p (cc_reg_1, prev))
+	{
+	  if (targetm.sched.macro_fusion_pair_p (prev, insn))
+	    SCHED_GROUP_P (insn) = 1;
+	  return;
+	}
     }
-  else
-    {
-      rtx insn_set = single_set (insn);
-
-      prev = prev_nonnote_nondebug_insn (insn);
-      if (!prev
-          || !insn_set
-          || !single_set (prev))
-        return;
 
+  if (single_set (insn) && single_set (prev))
+    {
+      if (targetm.sched.macro_fusion_pair_p (prev, insn))
+	SCHED_GROUP_P (insn) = 1;
     }
-
-  if (targetm.sched.macro_fusion_pair_p (prev, insn))
-    SCHED_GROUP_P (insn) = 1;
-
 }
 
 /* Get the implicit reg pending clobbers for INSN and save them in TEMP.  */

Re: [PING][PATCH] Move the check for any_condjump_p from sched-deps to target macros

[Jeff Law]

On 06/26/2017 10:19 PM, Hurugalawadi, Naveen wrote:
> Hi Jeff,
> 
> Thanks for the review and your approval for final patch.
> Sorry, It was a long weekend and hence could not revert to your
> comments earlier.
> 
>>> You need a ChangeLog entry, but I think that's it.  Can you
>>> please repost with a ChangeLog entry for final approval?
> 
> Please find the final patch and ChangeLog entry updated as required.
> Please review the same and let me know if its okay to commit?
> 
> Thanks,
> Naveen
> 
> 2017-06-27  Naveen H.S  <Naveen.Hurugalawadi@cavium.com>
> 
> 	* config/aarch64/aarch64.c (aarch_macro_fusion_pair_p): Push the
> 	check for CC usage into AARCH64_FUSE_CMP_BRANCH.
> 	* config/i386/i386.c (ix86_macro_fusion_pair_p): Push the check for
> 	CC usage from generic code to here.
> 	* sched-deps.c (sched_macro_fuse_insns): Move the condition for
> 	CC usage into the target macros.    
> 
OK for the trunk.  Thanks for your patience.

Jeff