public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
@ 2014-11-12 10:02 Yangfei (Felix)
  2014-11-12 10:55 ` Jiong Wang
  2014-11-12 16:06 ` Richard Henderson
  0 siblings, 2 replies; 14+ messages in thread
From: Yangfei (Felix) @ 2014-11-12 10:02 UTC (permalink / raw)
  To: Yangfei (Felix), gcc-patches, Marcus Shawcroft, Richard Earnshaw

Hello,

   Any comments on this patch?  Thanks.


> Hi,
>     This patch adds doloop_end standard pattern for AArch64 port so that
> -fmodulo-sched can do its job.
>     Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?
> 
> 
> Index: gcc/ChangeLog
> =============================================================
> ======
> --- gcc/ChangeLog	(revision 216838)
> +++ gcc/ChangeLog	(working copy)
> @@ -1,3 +1,7 @@
> +2014-11-10  Felix Yang  <felix.yang@huawei.com>
> +
> +	* config/aarch64/aarch64.md (doloop_end): New pattern.
> +
>  2014-10-29  Richard Sandiford  <richard.sandiford@arm.com>
> 
>  	* addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c,
> Index: gcc/config/aarch64/aarch64.md
> =============================================================
> ======
> --- gcc/config/aarch64/aarch64.md	(revision 216838)
> +++ gcc/config/aarch64/aarch64.md	(working copy)
> @@ -4087,6 +4087,48 @@
>    [(set_attr "type" "mrs")])
> 
> 
> +;; Define the subtract-one-and-jump insns so loop.c ;; knows what to
> +generate.
> +(define_expand "doloop_end"
> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> +   (use (match_operand 1 "" ""))]     ; label
> +  ""
> +  "
> +{
> +  /* Currently SMS relies on the do-loop pattern to recognize loops
> +     where (1) the control part consists of all insns defining and/or
> +     using a certain 'count' register and (2) the loop count can be
> +     adjusted by modifying this register prior to the loop.
> +     ??? The possible introduction of a new block to initialize the
> +     new IV can potentially affect branch optimizations.  */
> +  if (optimize > 0 && flag_modulo_sched)
> +    {
> +      rtx s0;
> +      rtx bcomp;
> +      rtx loc_ref;
> +      rtx cc_reg;
> +      rtx insn;
> +      rtx cmp;
> +
> +      if (GET_MODE (operands[0]) != DImode)
> +	FAIL;
> +
> +      s0 = operands [0];
> +      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
> +
> +      cmp = XVECEXP (PATTERN (insn), 0, 0);
> +      cc_reg = SET_DEST (cmp);
> +      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
> +      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
> +      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
> +				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
> +							 loc_ref, pc_rtx)));
> +      DONE;
> +    }
> +  else
> +    FAIL;
> +}")
> +
>  ;; AdvSIMD Stuff
>  (include "aarch64-simd.md")


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-12 10:02 [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched Yangfei (Felix)
@ 2014-11-12 10:55 ` Jiong Wang
  2014-11-12 15:59   ` Andrew Pinski
  2014-11-12 16:06 ` Richard Henderson
  1 sibling, 1 reply; 14+ messages in thread
From: Jiong Wang @ 2014-11-12 10:55 UTC (permalink / raw)
  To: Yangfei (Felix), gcc-patches, Marcus Shawcroft, Richard Earnshaw

On 12/11/14 10:01, Yangfei (Felix) wrote:

> +      if (GET_MODE (operands[0]) != DImode)
> +	FAIL;

shouldn't SImode be considered for -mabi=ilp32 ?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-12 10:55 ` Jiong Wang
@ 2014-11-12 15:59   ` Andrew Pinski
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Pinski @ 2014-11-12 15:59 UTC (permalink / raw)
  To: Jiong Wang
  Cc: Yangfei (Felix), gcc-patches, Marcus Shawcroft, Richard Earnshaw

On Wed, Nov 12, 2014 at 2:54 AM, Jiong Wang <jiong.wang@arm.com> wrote:
> On 12/11/14 10:01, Yangfei (Felix) wrote:
>
>> +      if (GET_MODE (operands[0]) != DImode)
>> +       FAIL;
>
>
> shouldn't SImode be considered for -mabi=ilp32 ?

I don't think it is needed for -mabi=ilp32 since word_mode is still
DImode for -mabi=ilp32.

Thanks,
Andrew


>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-12 10:02 [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched Yangfei (Felix)
  2014-11-12 10:55 ` Jiong Wang
@ 2014-11-12 16:06 ` Richard Henderson
  2014-11-13 10:45   ` [PING ^ 2][PATCH, " Yangfei (Felix)
  2014-11-17  8:57   ` [PING ^ 3][PATCH, " Yangfei (Felix)
  1 sibling, 2 replies; 14+ messages in thread
From: Richard Henderson @ 2014-11-12 16:06 UTC (permalink / raw)
  To: Yangfei (Felix), gcc-patches, Marcus Shawcroft, Richard Earnshaw

On 11/12/2014 11:01 AM, Yangfei (Felix) wrote:
> +(define_expand "doloop_end"
>> +  [(use (match_operand 0 "" ""))      ; loop pseudo
>> +   (use (match_operand 1 "" ""))]     ; label
>> +  ""
>> +  "
>> +{

Drop the "" surrounding the { }.


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PING ^ 2][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-12 16:06 ` Richard Henderson
@ 2014-11-13 10:45   ` Yangfei (Felix)
  2014-11-17  8:57   ` [PING ^ 3][PATCH, " Yangfei (Felix)
  1 sibling, 0 replies; 14+ messages in thread
From: Yangfei (Felix) @ 2014-11-13 10:45 UTC (permalink / raw)
  To: Richard Henderson, gcc-patches, Marcus Shawcroft, Richard Earnshaw

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

> On 11/12/2014 11:01 AM, Yangfei (Felix) wrote:
> > +(define_expand "doloop_end"
> >> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> >> +   (use (match_operand 1 "" ""))]     ; label
> >> +  ""
> >> +  "
> >> +{
> 
> Drop the "" surrounding the { }.
> 
> 
> r~


Hello,
    I updated the patch with the redundant "" removed.  Is it OK for trunk now? 
    I hope this patch can catch up with stage 1 of GCC-5.0.  Thanks. 


Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.md (doloop_end): New pattern.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,47 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  ""
+{
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+  if (optimize > 0 && flag_modulo_sched)
+    {
+      rtx s0;
+      rtx bcomp;
+      rtx loc_ref;
+      rtx cc_reg;
+      rtx insn;
+      rtx cmp;
+
+      if (GET_MODE (operands[0]) != DImode)
+	FAIL;
+
+      s0 = operands [0];
+      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+      cmp = XVECEXP (PATTERN (insn), 0, 0);
+      cc_reg = SET_DEST (cmp);
+      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+							 loc_ref, pc_rtx)));
+      DONE;
+    }
+  else
+    FAIL;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")


[-- Attachment #2: aarch64-doloop-v2.diff --]
[-- Type: application/octet-stream, Size: 1966 bytes --]

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.md (doloop_end): New pattern.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,47 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  ""
+{
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+  if (optimize > 0 && flag_modulo_sched)
+    {
+      rtx s0;
+      rtx bcomp;
+      rtx loc_ref;
+      rtx cc_reg;
+      rtx insn;
+      rtx cmp;
+
+      if (GET_MODE (operands[0]) != DImode)
+	FAIL;
+
+      s0 = operands [0];
+      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+      cmp = XVECEXP (PATTERN (insn), 0, 0);
+      cc_reg = SET_DEST (cmp);
+      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+							 loc_ref, pc_rtx)));
+      DONE;
+    }
+  else
+    FAIL;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-12 16:06 ` Richard Henderson
  2014-11-13 10:45   ` [PING ^ 2][PATCH, " Yangfei (Felix)
@ 2014-11-17  8:57   ` Yangfei (Felix)
  2014-11-18 10:18     ` Marcus Shawcroft
  1 sibling, 1 reply; 14+ messages in thread
From: Yangfei (Felix) @ 2014-11-17  8:57 UTC (permalink / raw)
  To: gcc-patches, Marcus Shawcroft, marcus.shawcroft, Richard Earnshaw

PING?  Is it OK for me to apply this patch?  Thanks.


> 
> > On 11/12/2014 11:01 AM, Yangfei (Felix) wrote:
> > > +(define_expand "doloop_end"
> > >> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> > >> +   (use (match_operand 1 "" ""))]     ; label
> > >> +  ""
> > >> +  "
> > >> +{
> >
> > Drop the "" surrounding the { }.
> >
> >
> > r~
> 
> 
> Hello,
>     I updated the patch with the redundant "" removed.  Is it OK for trunk
> now?
>     I hope this patch can catch up with stage 1 of GCC-5.0.  Thanks.
> 
> 
> Index: gcc/ChangeLog
> =============================================================
> ======
> --- gcc/ChangeLog	(revision 217394)
> +++ gcc/ChangeLog	(working copy)
> @@ -1,3 +1,7 @@
> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> +
> +	* config/aarch64/aarch64.md (doloop_end): New pattern.
> +
>  2014-11-11  Andrew Pinski  <apinski@cavium.com>
> 
>  	Bug target/61997
> Index: gcc/config/aarch64/aarch64.md
> =============================================================
> ======
> --- gcc/config/aarch64/aarch64.md	(revision 217394)
> +++ gcc/config/aarch64/aarch64.md	(working copy)
> @@ -4087,6 +4087,47 @@
>    [(set_attr "type" "mrs")])
> 
> 
> +;; Define the subtract-one-and-jump insns so loop.c ;; knows what to
> +generate.
> +(define_expand "doloop_end"
> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> +   (use (match_operand 1 "" ""))]     ; label
> +  ""
> +{
> +  /* Currently SMS relies on the do-loop pattern to recognize loops
> +     where (1) the control part consists of all insns defining and/or
> +     using a certain 'count' register and (2) the loop count can be
> +     adjusted by modifying this register prior to the loop.
> +     ??? The possible introduction of a new block to initialize the
> +     new IV can potentially affect branch optimizations.  */
> +  if (optimize > 0 && flag_modulo_sched)
> +    {
> +      rtx s0;
> +      rtx bcomp;
> +      rtx loc_ref;
> +      rtx cc_reg;
> +      rtx insn;
> +      rtx cmp;
> +
> +      if (GET_MODE (operands[0]) != DImode)
> +	FAIL;
> +
> +      s0 = operands [0];
> +      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
> +
> +      cmp = XVECEXP (PATTERN (insn), 0, 0);
> +      cc_reg = SET_DEST (cmp);
> +      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
> +      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
> +      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
> +				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
> +							 loc_ref, pc_rtx)));
> +      DONE;
> +    }
> +  else
> +    FAIL;
> +})
> +
>  ;; AdvSIMD Stuff
>  (include "aarch64-simd.md")


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-17  8:57   ` [PING ^ 3][PATCH, " Yangfei (Felix)
@ 2014-11-18 10:18     ` Marcus Shawcroft
  2014-11-18 10:56       ` Yangfei (Felix)
  0 siblings, 1 reply; 14+ messages in thread
From: Marcus Shawcroft @ 2014-11-18 10:18 UTC (permalink / raw)
  To: Yangfei (Felix); +Cc: gcc-patches, marcus.shawcroft, Richard Earnshaw

On 17 November 2014 07:59, Yangfei (Felix) <felix.yang@huawei.com> wrote:

>> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
>> +
>> +     * config/aarch64/aarch64.md (doloop_end): New pattern.
>> +

This looks like a straight copy of the ARM code, but without the
TARGET_CAN_USE_DOLOOP_P definition.  If the reason for including this
code is for the benefit of module-sched then should the hook be
defined to limit the use of this pattern to inner most loops only?

Cheers
/Marcus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 10:18     ` Marcus Shawcroft
@ 2014-11-18 10:56       ` Yangfei (Felix)
  2014-11-18 11:29         ` Richard Henderson
  0 siblings, 1 reply; 14+ messages in thread
From: Yangfei (Felix) @ 2014-11-18 10:56 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: gcc-patches, marcus.shawcroft, Richard Earnshaw

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

Yes, major code is borrowed from ARM port with the expected mode of loop pseudo changed to DImode. 
The function loop_canon_p called in sms_schedule can make sure that SMS is only performed for innermost loops. 
But I think it's a good idea to implement the TARGET_CAN_USE_DOLOOP_P hook here. 
See my updated patch below.  How about this one? 


Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.c (doloop_end): New pattern.
+	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,47 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  ""
+{
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+  if (optimize > 0 && flag_modulo_sched)
+    {
+      rtx s0;
+      rtx bcomp;
+      rtx loc_ref;
+      rtx cc_reg;
+      rtx insn;
+      rtx cmp;
+
+      if (GET_MODE (operands[0]) != DImode)
+	FAIL;
+
+      s0 = operands [0];
+      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+      cmp = XVECEXP (PATTERN (insn), 0, 0);
+      cc_reg = SET_DEST (cmp);
+      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+							 loc_ref, pc_rtx)));
+      DONE;
+    }
+  else
+    FAIL;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	(revision 217394)
+++ gcc/config/aarch64/aarch64.c	(working copy)
@@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p (unsigned i
 #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
   aarch64_use_by_pieces_infrastructure_p
 
+#undef TARGET_CAN_USE_DOLOOP_P
+#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"


> 
> On 17 November 2014 07:59, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> 
> >> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> >> +
> >> +     * config/aarch64/aarch64.md (doloop_end): New pattern.
> >> +
> 
> This looks like a straight copy of the ARM code, but without the
> TARGET_CAN_USE_DOLOOP_P definition.  If the reason for including this code
> is for the benefit of module-sched then should the hook be defined to limit the
> use of this pattern to inner most loops only?
> 
> Cheers
> /Marcus

[-- Attachment #2: aarch64-doloop-v4.diff --]
[-- Type: application/octet-stream, Size: 2575 bytes --]

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.c (doloop_end): New pattern.
+	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,47 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  ""
+{
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+  if (optimize > 0 && flag_modulo_sched)
+    {
+      rtx s0;
+      rtx bcomp;
+      rtx loc_ref;
+      rtx cc_reg;
+      rtx insn;
+      rtx cmp;
+
+      if (GET_MODE (operands[0]) != DImode)
+	FAIL;
+
+      s0 = operands [0];
+      insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+      cmp = XVECEXP (PATTERN (insn), 0, 0);
+      cc_reg = SET_DEST (cmp);
+      bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+      loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+      emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+				   gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+							 loc_ref, pc_rtx)));
+      DONE;
+    }
+  else
+    FAIL;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	(revision 217394)
+++ gcc/config/aarch64/aarch64.c	(working copy)
@@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p (unsigned i
 #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
   aarch64_use_by_pieces_infrastructure_p
 
+#undef TARGET_CAN_USE_DOLOOP_P
+#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 10:56       ` Yangfei (Felix)
@ 2014-11-18 11:29         ` Richard Henderson
  2014-11-18 11:51           ` Yangfei (Felix)
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2014-11-18 11:29 UTC (permalink / raw)
  To: Yangfei (Felix), Marcus Shawcroft
  Cc: gcc-patches, marcus.shawcroft, Richard Earnshaw

On 11/18/2014 11:48 AM, Yangfei (Felix) wrote:
> +(define_expand "doloop_end"
> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> +   (use (match_operand 1 "" ""))]     ; label
> +  ""
> +{
> +  /* Currently SMS relies on the do-loop pattern to recognize loops
> +     where (1) the control part consists of all insns defining and/or
> +     using a certain 'count' register and (2) the loop count can be
> +     adjusted by modifying this register prior to the loop.
> +     ??? The possible introduction of a new block to initialize the
> +     new IV can potentially affect branch optimizations.  */
> +  if (optimize > 0 && flag_modulo_sched)

You'd be better off moving this condition into the expansion predicate (which
is currently "").

This short-circuits a lot of unnecessary work.  See  pass_rtl_doloop::gate.


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 11:29         ` Richard Henderson
@ 2014-11-18 11:51           ` Yangfei (Felix)
  2014-11-18 11:59             ` Richard Henderson
                               ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Yangfei (Felix) @ 2014-11-18 11:51 UTC (permalink / raw)
  To: Richard Henderson, Marcus Shawcroft
  Cc: gcc-patches, marcus.shawcroft, Richard Earnshaw

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

> On 11/18/2014 11:48 AM, Yangfei (Felix) wrote:
> > +(define_expand "doloop_end"
> > +  [(use (match_operand 0 "" ""))      ; loop pseudo
> > +   (use (match_operand 1 "" ""))]     ; label
> > +  ""
> > +{
> > +  /* Currently SMS relies on the do-loop pattern to recognize loops
> > +     where (1) the control part consists of all insns defining and/or
> > +     using a certain 'count' register and (2) the loop count can be
> > +     adjusted by modifying this register prior to the loop.
> > +     ??? The possible introduction of a new block to initialize the
> > +     new IV can potentially affect branch optimizations.  */
> > +  if (optimize > 0 && flag_modulo_sched)
> 
> You'd be better off moving this condition into the expansion predicate (which is
> currently "").
> 
> This short-circuits a lot of unnecessary work.  See  pass_rtl_doloop::gate.
> 
> 
> r~


Yeah, that's a good idea.  See my updated patch :-)


Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.c (doloop_end): New pattern.
+	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,43 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  "optimize > 0 && flag_modulo_sched"
+{
+  rtx s0;
+  rtx bcomp;
+  rtx loc_ref;
+  rtx cc_reg;
+  rtx insn;
+  rtx cmp;
+
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+
+  if (GET_MODE (operands[0]) != DImode)
+    FAIL;
+
+  s0 = operands [0];
+  insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+  cmp = XVECEXP (PATTERN (insn), 0, 0);
+  cc_reg = SET_DEST (cmp);
+  bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+  loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+  emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+			       gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+						     loc_ref, pc_rtx)));
+  DONE;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	(revision 217394)
+++ gcc/config/aarch64/aarch64.c	(working copy)
@@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p (unsigned i
 #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
   aarch64_use_by_pieces_infrastructure_p
 
+#undef TARGET_CAN_USE_DOLOOP_P
+#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"


[-- Attachment #2: aarch64-doloop-v5.diff --]
[-- Type: application/octet-stream, Size: 2484 bytes --]

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+
+	* config/aarch64/aarch64.c (doloop_end): New pattern.
+	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217394)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -4087,6 +4087,43 @@
   [(set_attr "type" "mrs")])
 
 
+;; Define the subtract-one-and-jump insns so loop.c
+;; knows what to generate.
+(define_expand "doloop_end"
+  [(use (match_operand 0 "" ""))      ; loop pseudo
+   (use (match_operand 1 "" ""))]     ; label
+  "optimize > 0 && flag_modulo_sched"
+{
+  rtx s0;
+  rtx bcomp;
+  rtx loc_ref;
+  rtx cc_reg;
+  rtx insn;
+  rtx cmp;
+
+  /* Currently SMS relies on the do-loop pattern to recognize loops
+     where (1) the control part consists of all insns defining and/or
+     using a certain 'count' register and (2) the loop count can be
+     adjusted by modifying this register prior to the loop.
+     ??? The possible introduction of a new block to initialize the
+     new IV can potentially affect branch optimizations.  */
+
+  if (GET_MODE (operands[0]) != DImode)
+    FAIL;
+
+  s0 = operands [0];
+  insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
+
+  cmp = XVECEXP (PATTERN (insn), 0, 0);
+  cc_reg = SET_DEST (cmp);
+  bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
+  loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
+  emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+			       gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+						     loc_ref, pc_rtx)));
+  DONE;
+})
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	(revision 217394)
+++ gcc/config/aarch64/aarch64.c	(working copy)
@@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p (unsigned i
 #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
   aarch64_use_by_pieces_infrastructure_p
 
+#undef TARGET_CAN_USE_DOLOOP_P
+#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 11:51           ` Yangfei (Felix)
@ 2014-11-18 11:59             ` Richard Henderson
  2014-11-18 12:02             ` Marcus Shawcroft
  2014-12-09 14:35             ` Tejas Belagod
  2 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2014-11-18 11:59 UTC (permalink / raw)
  To: Yangfei (Felix), Marcus Shawcroft
  Cc: gcc-patches, marcus.shawcroft, Richard Earnshaw

On 11/18/2014 12:28 PM, Yangfei (Felix) wrote:
> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> +
> +	* config/aarch64/aarch64.c (doloop_end): New pattern.
> +	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.

Looks good to me.  I'll leave it for aarch64 maintainers for final approval.


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 11:51           ` Yangfei (Felix)
  2014-11-18 11:59             ` Richard Henderson
@ 2014-11-18 12:02             ` Marcus Shawcroft
  2014-12-09 14:35             ` Tejas Belagod
  2 siblings, 0 replies; 14+ messages in thread
From: Marcus Shawcroft @ 2014-11-18 12:02 UTC (permalink / raw)
  To: Yangfei (Felix)
  Cc: Richard Henderson, gcc-patches, marcus.shawcroft, Richard Earnshaw

On 18 November 2014 11:28, Yangfei (Felix) <felix.yang@huawei.com> wrote:

> Yeah, that's a good idea.  See my updated patch :-)
>
>
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog       (revision 217394)
> +++ gcc/ChangeLog       (working copy)
> @@ -1,3 +1,8 @@
> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> +
> +       * config/aarch64/aarch64.c (doloop_end): New pattern.
> +       * config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
> +


OK with me /Marcus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-11-18 11:51           ` Yangfei (Felix)
  2014-11-18 11:59             ` Richard Henderson
  2014-11-18 12:02             ` Marcus Shawcroft
@ 2014-12-09 14:35             ` Tejas Belagod
  2014-12-10  8:23               ` Yangfei (Felix)
  2 siblings, 1 reply; 14+ messages in thread
From: Tejas Belagod @ 2014-12-09 14:35 UTC (permalink / raw)
  To: Yangfei (Felix)
  Cc: Richard Henderson, Marcus Shawcroft, gcc-patches,
	Marcus Shawcroft, Richard Earnshaw

On 18/11/14 11:28, Yangfei (Felix) wrote:
>> On 11/18/2014 11:48 AM, Yangfei (Felix) wrote:
>>> +(define_expand "doloop_end"
>>> +  [(use (match_operand 0 "" ""))      ; loop pseudo
>>> +   (use (match_operand 1 "" ""))]     ; label
>>> +  ""
>>> +{
>>> +  /* Currently SMS relies on the do-loop pattern to recognize loops
>>> +     where (1) the control part consists of all insns defining and/or
>>> +     using a certain 'count' register and (2) the loop count can be
>>> +     adjusted by modifying this register prior to the loop.
>>> +     ??? The possible introduction of a new block to initialize the
>>> +     new IV can potentially affect branch optimizations.  */
>>> +  if (optimize > 0 && flag_modulo_sched)
>>
>> You'd be better off moving this condition into the expansion predicate (which is
>> currently "").
>>
>> This short-circuits a lot of unnecessary work.  See  pass_rtl_doloop::gate.
>>
>>
>> r~
>
>
> Yeah, that's a good idea.  See my updated patch :-)
>
>
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog	(revision 217394)
> +++ gcc/ChangeLog	(working copy)
> @@ -1,3 +1,8 @@
> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> +
> +	* config/aarch64/aarch64.c (doloop_end): New pattern.
> +	* config/aarch64/aarch64.md (TARGET_CAN_USE_DOLOOP_P): Implement.
> +
>   2014-11-11  Andrew Pinski  <apinski@cavium.com>
>
>   	Bug target/61997
> Index: gcc/config/aarch64/aarch64.md
> ===================================================================
> --- gcc/config/aarch64/aarch64.md	(revision 217394)
> +++ gcc/config/aarch64/aarch64.md	(working copy)
> @@ -4087,6 +4087,43 @@
>     [(set_attr "type" "mrs")])
>
>
> +;; Define the subtract-one-and-jump insns so loop.c
> +;; knows what to generate.
> +(define_expand "doloop_end"
> +  [(use (match_operand 0 "" ""))      ; loop pseudo
> +   (use (match_operand 1 "" ""))]     ; label
> +  "optimize > 0 && flag_modulo_sched"
> +{
> +  rtx s0;
> +  rtx bcomp;
> +  rtx loc_ref;
> +  rtx cc_reg;
> +  rtx insn;
> +  rtx cmp;
> +
> +  /* Currently SMS relies on the do-loop pattern to recognize loops
> +     where (1) the control part consists of all insns defining and/or
> +     using a certain 'count' register and (2) the loop count can be
> +     adjusted by modifying this register prior to the loop.
> +     ??? The possible introduction of a new block to initialize the
> +     new IV can potentially affect branch optimizations.  */
> +
> +  if (GET_MODE (operands[0]) != DImode)
> +    FAIL;
> +
> +  s0 = operands [0];
> +  insn = emit_insn (gen_adddi3_compare0 (s0, s0, GEN_INT (-1)));
> +
> +  cmp = XVECEXP (PATTERN (insn), 0, 0);
> +  cc_reg = SET_DEST (cmp);
> +  bcomp = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
> +  loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands [1]);
> +  emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
> +			       gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
> +						     loc_ref, pc_rtx)));
> +  DONE;
> +})
> +
>   ;; AdvSIMD Stuff
>   (include "aarch64-simd.md")
>
> Index: gcc/config/aarch64/aarch64.c
> ===================================================================
> --- gcc/config/aarch64/aarch64.c	(revision 217394)
> +++ gcc/config/aarch64/aarch64.c	(working copy)
> @@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p (unsigned i
>   #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
>     aarch64_use_by_pieces_infrastructure_p
>
> +#undef TARGET_CAN_USE_DOLOOP_P
> +#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
> +
>   struct gcc_target targetm = TARGET_INITIALIZER;
>
>   #include "gt-aarch64.h"
>

Hi Felix,

This patch causes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64240 
when sms-3 is tested with -fPIC. It runs fine when I reverse this patch out.

Please could you have a look?

Thanks,
Tejas.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING ^ 3][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched
  2014-12-09 14:35             ` Tejas Belagod
@ 2014-12-10  8:23               ` Yangfei (Felix)
  0 siblings, 0 replies; 14+ messages in thread
From: Yangfei (Felix) @ 2014-12-10  8:23 UTC (permalink / raw)
  To: Tejas Belagod; +Cc: gcc-patches, Marcus Shawcroft, Richard Earnshaw

> > --- gcc/config/aarch64/aarch64.c	(revision 217394)
> > +++ gcc/config/aarch64/aarch64.c	(working copy)
> > @@ -10224,6 +10224,9 @@ aarch64_use_by_pieces_infrastructure_p
> (unsigned i
> >   #define TARGET_USE_BY_PIECES_INFRASTRUCTURE_P \
> >     aarch64_use_by_pieces_infrastructure_p
> >
> > +#undef TARGET_CAN_USE_DOLOOP_P
> > +#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
> > +
> >   struct gcc_target targetm = TARGET_INITIALIZER;
> >
> >   #include "gt-aarch64.h"
> >
> 
> Hi Felix,
> 
> This patch causes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64240
> when sms-3 is tested with -fPIC. It runs fine when I reverse this patch out.
> 
> Please could you have a look?
> 
> Thanks,
> Tejas.

OK, I have reproduced with -fPIC option. Will take a look.  


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-12-10  8:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-12 10:02 [PING][PATCH, AArch64] Add doloop_end pattern for -fmodulo-sched Yangfei (Felix)
2014-11-12 10:55 ` Jiong Wang
2014-11-12 15:59   ` Andrew Pinski
2014-11-12 16:06 ` Richard Henderson
2014-11-13 10:45   ` [PING ^ 2][PATCH, " Yangfei (Felix)
2014-11-17  8:57   ` [PING ^ 3][PATCH, " Yangfei (Felix)
2014-11-18 10:18     ` Marcus Shawcroft
2014-11-18 10:56       ` Yangfei (Felix)
2014-11-18 11:29         ` Richard Henderson
2014-11-18 11:51           ` Yangfei (Felix)
2014-11-18 11:59             ` Richard Henderson
2014-11-18 12:02             ` Marcus Shawcroft
2014-12-09 14:35             ` Tejas Belagod
2014-12-10  8:23               ` Yangfei (Felix)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).