[PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Evandro Menezes]

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

In some micro-architectures the insns to load or store pairs of vector 
registers are implemented rather differently from those affecting lanes 
in vector registers.  Then, it's important that such insns be described 
likewise differently in the scheduling model.

This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart 
from the current neon_load2_2reg_q and neon_store2_2reg_q types, 
respectively.

Thank you,

-- 
Evandro Menezes


[-- Attachment #2: 0001-AArch64-Add-separate-insn-sched-class-for-vector-LDP.patch --]
[-- Type: text/x-patch, Size: 5067 bytes --]

From 340249dcd2af8dfce486cb4f62d4eaf285c6a799 Mon Sep 17 00:00:00 2001
From: Evandro Menezes <e.menezes@samsung.com>
Date: Mon, 28 Sep 2015 15:00:00 -0500
Subject: [PATCH] [AArch64] Add separate insn sched class for vector LDP & STP

2015-09-28  Evandro Menezes  <e.menezes@samsung.com>

	gcc/
	* config/arm/types.md (neon_ldp, neon_ldp_q, neon_stp, neon_stp_q):
	add new insn types for vector load and store pairs.
	* config/arm/cortex-a53.md (cortex_a53_f_load_2reg): add insn
	types "neon_ldp{,_q}".
	* config/arm/cortex-a57.md (neon_load_c): add insn types
	"neon_ldp{,_q}".
	(neon_store_complex): add insn types "neon_stp{,_q}".
	* config/aarch64/aarch64-simd.md (aarch64_be_movoi): add insn types
	"neon_{ldp,stp}_q".
---
 gcc/config/aarch64/aarch64-simd.md |  2 +-
 gcc/config/arm/cortex-a53.md       |  2 +-
 gcc/config/arm/cortex-a57.md       |  6 ++++--
 gcc/config/arm/types.md            | 14 ++++++++++++--
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 5ab2f2b..541faf9 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -4327,7 +4327,7 @@
    #
    stp\\t%q1, %R1, %0
    ldp\\t%q0, %R0, %1"
-  [(set_attr "type" "multiple,neon_store2_2reg_q,neon_load2_2reg_q")
+  [(set_attr "type" "multiple,neon_stp_q,neon_ldp_q")
    (set (attr "length") (symbol_ref "aarch64_simd_attr_length_move (insn)"))]
 )
 
diff --git a/gcc/config/arm/cortex-a53.md b/gcc/config/arm/cortex-a53.md
index 3fa0625..032d5eb 100644
--- a/gcc/config/arm/cortex-a53.md
+++ b/gcc/config/arm/cortex-a53.md
@@ -325,7 +325,7 @@
 
 (define_insn_reservation "cortex_a53_f_load_2reg" 5
   (and (eq_attr "tune" "cortexa53")
-       (eq_attr "type" "neon_load2_2reg_q"))
+       (eq_attr "type" "neon_ldp, neon_ldp_q, neon_load2_2reg_q"))
   "(cortex_a53_slot_any+cortex_a53_ls)*2")
 
 (define_insn_reservation "cortex_a53_f_loadq" 5
diff --git a/gcc/config/arm/cortex-a57.md b/gcc/config/arm/cortex-a57.md
index d6ce440..c751dd4 100644
--- a/gcc/config/arm/cortex-a57.md
+++ b/gcc/config/arm/cortex-a57.md
@@ -202,7 +202,8 @@
 	  (eq_attr "type" "neon_load1_3reg, neon_load1_3reg_q,\
 			   neon_load1_4reg, neon_load1_4reg_q")
 	    (const_string "neon_load_b")
-	  (eq_attr "type" "neon_load1_one_lane, neon_load1_one_lane_q,\
+	  (eq_attr "type" "neon_ldp, neon_ldp_q,\
+			   neon_load1_one_lane, neon_load1_one_lane_q,\
 			   neon_load1_all_lanes, neon_load1_all_lanes_q,\
 			   neon_load2_2reg, neon_load2_2reg_q,\
 			   neon_load2_all_lanes, neon_load2_all_lanes_q")
@@ -224,7 +225,8 @@
 	    (const_string "neon_store_a")
 	  (eq_attr "type" "neon_store1_2reg, neon_store1_1reg_q")
 	    (const_string "neon_store_b")
-	  (eq_attr "type" "neon_store1_3reg, neon_store1_3reg_q,\
+	  (eq_attr "type" "neon_stp, neon_stp_q,\
+			   neon_store1_3reg, neon_store1_3reg_q,\
 			   neon_store3_3reg, neon_store3_3reg_q,\
 			   neon_store2_4reg, neon_store2_4reg_q,\
 			   neon_store4_4reg, neon_store4_4reg_q,\
diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md
index 534be74..73f482d 100644
--- a/gcc/config/arm/types.md
+++ b/gcc/config/arm/types.md
@@ -376,6 +376,8 @@
 ; neon_from_gp
 ; neon_from_gp_q
 ; neon_ldr
+; neon_ldp
+; neon_ldp_q
 ; neon_load1_1reg
 ; neon_load1_1reg_q
 ; neon_load1_2reg
@@ -409,6 +411,8 @@
 ; neon_load4_one_lane
 ; neon_load4_one_lane_q
 ; neon_str
+; neon_stp
+; neon_stp_q
 ; neon_store1_1reg
 ; neon_store1_1reg_q
 ; neon_store1_2reg
@@ -889,6 +893,8 @@
   neon_from_gp_q,\
 \
   neon_ldr,\
+  neon_ldp,\
+  neon_ldp_q,\
   neon_load1_1reg,\
   neon_load1_1reg_q,\
   neon_load1_2reg,\
@@ -926,6 +932,8 @@
   neon_load4_one_lane_q,\
 \
   neon_str,\
+  neon_stp,\
+  neon_stp_q,\
   neon_store1_1reg,\
   neon_store1_1reg_q,\
   neon_store1_2reg,\
@@ -1128,7 +1136,8 @@
           neon_sat_mla_s_long, neon_sat_mla_h_scalar_long,\
           neon_sat_mla_s_scalar_long,\
           neon_to_gp, neon_to_gp_q, neon_from_gp, neon_from_gp_q,\
-          neon_ldr, neon_load1_1reg, neon_load1_1reg_q, neon_load1_2reg,\
+	   neon_ldr, neon_ldp, neon_ldp_q,\
+	   neon_load1_1reg, neon_load1_1reg_q, neon_load1_2reg,\
           neon_load1_2reg_q, neon_load1_3reg, neon_load1_3reg_q,\
           neon_load1_4reg, neon_load1_4reg_q, neon_load1_all_lanes,\
           neon_load1_all_lanes_q, neon_load1_one_lane, neon_load1_one_lane_q,\
@@ -1139,7 +1148,8 @@
           neon_load3_all_lanes_q, neon_load3_one_lane, neon_load3_one_lane_q,\
           neon_load4_4reg, neon_load4_4reg_q, neon_load4_all_lanes,\
           neon_load4_all_lanes_q, neon_load4_one_lane, neon_load4_one_lane_q,\
-          neon_str, neon_store1_1reg, neon_store1_1reg_q, neon_store1_2reg,\
+	   neon_str, neon_stp, neon_stp_q,\
+	   neon_store1_1reg, neon_store1_1reg_q, neon_store1_2reg,\
           neon_store1_2reg_q, neon_store1_3reg, neon_store1_3reg_q,\
           neon_store1_4reg, neon_store1_4reg_q, neon_store1_one_lane,\
           neon_store1_one_lane_q, neon_store2_2reg, neon_store2_2reg_q,\
-- 
2.1.0.243.g30d45f7

Re: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Andrew Pinski]

On Mon, Sep 28, 2015 at 4:52 PM, Evandro Menezes <e.menezes@samsung.com> wrote:
> In some micro-architectures the insns to load or store pairs of vector
> registers are implemented rather differently from those affecting lanes in
> vector registers.  Then, it's important that such insns be described
> likewise differently in the scheduling model.
>
> This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart from
> the current neon_load2_2reg_q and neon_store2_2reg_q types, respectively.

This is a very useful patch for the ThunderX core also.  I will update
the config/aarch64/thunderx.md file if this patch gets approved.

Thanks,
Andrew

>
> Thank you,
>
> --
> Evandro Menezes
>

Re: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Marcus Shawcroft]

On 29/09/15 00:52, Evandro Menezes wrote:
> In some micro-architectures the insns to load or store pairs of vector
> registers are implemented rather differently from those affecting lanes
> in vector registers.  Then, it's important that such insns be described
> likewise differently in the scheduling model.
>
> This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart
> from the current neon_load2_2reg_q and neon_store2_2reg_q types,
> respectively.
>

Hi,

The AArch64 part of this is OK. Please wait for Kyrill or Ramana to 
comment on ARM side.  Cheers /Marcus

> Thank you,
>
> -- Evandro Menezes
>
>
> 0001-AArch64-Add-separate-insn-sched-class-for-vector-LDP.patch
>
>
>  From 340249dcd2af8dfce486cb4f62d4eaf285c6a799 Mon Sep 17 00:00:00 2001
> From: Evandro Menezes<e.menezes@samsung.com>
> Date: Mon, 28 Sep 2015 15:00:00 -0500
> Subject: [PATCH] [AArch64] Add separate insn sched class for vector LDP & STP
>
> 2015-09-28  Evandro Menezes<e.menezes@samsung.com>
>
> 	gcc/
> 	* config/arm/types.md (neon_ldp, neon_ldp_q, neon_stp, neon_stp_q):
> 	add new insn types for vector load and store pairs.

s/add/Add/ and likewise the rest of the changelog comments.

> 	* config/arm/cortex-a53.md (cortex_a53_f_load_2reg): add insn
> 	types "neon_ldp{,_q}".
> 	* config/arm/cortex-a57.md (neon_load_c): add insn types
> 	"neon_ldp{,_q}".
> 	(neon_store_complex): add insn types "neon_stp{,_q}".
> 	* config/aarch64/aarch64-simd.md (aarch64_be_movoi): add insn types
> 	"neon_{ldp,stp}_q".

Re: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Kyrill Tkachov]

On 29/09/15 09:03, Marcus Shawcroft wrote:
> On 29/09/15 00:52, Evandro Menezes wrote:
>> In some micro-architectures the insns to load or store pairs of vector
>> registers are implemented rather differently from those affecting lanes
>> in vector registers.  Then, it's important that such insns be described
>> likewise differently in the scheduling model.
>>
>> This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart
>> from the current neon_load2_2reg_q and neon_store2_2reg_q types,
>> respectively.
>>
> Hi,
>
> The AArch64 part of this is OK. Please wait for Kyrill or Ramana to
> comment on ARM side.  Cheers /Marcus
>

This is ok arm-wise. I see the instructions being modelled
with this type don't have a direct arm equivalent anyway.
Marcus' comment on the ChangeLog still apply.

Thanks,
Kyrill

>> Thank you,
>>
>> -- Evandro Menezes
>>
>>
>> 0001-AArch64-Add-separate-insn-sched-class-for-vector-LDP.patch
>>
>>
>>   From 340249dcd2af8dfce486cb4f62d4eaf285c6a799 Mon Sep 17 00:00:00 2001
>> From: Evandro Menezes<e.menezes@samsung.com>
>> Date: Mon, 28 Sep 2015 15:00:00 -0500
>> Subject: [PATCH] [AArch64] Add separate insn sched class for vector LDP & STP
>>
>> 2015-09-28  Evandro Menezes<e.menezes@samsung.com>
>>
>> 	gcc/
>> 	* config/arm/types.md (neon_ldp, neon_ldp_q, neon_stp, neon_stp_q):
>> 	add new insn types for vector load and store pairs.
> s/add/Add/ and likewise the rest of the changelog comments.
>
>> 	* config/arm/cortex-a53.md (cortex_a53_f_load_2reg): add insn
>> 	types "neon_ldp{,_q}".
>> 	* config/arm/cortex-a57.md (neon_load_c): add insn types
>> 	"neon_ldp{,_q}".
>> 	(neon_store_complex): add insn types "neon_stp{,_q}".
>> 	* config/aarch64/aarch64-simd.md (aarch64_be_movoi): add insn types
>> 	"neon_{ldp,stp}_q".

RE: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Evandro Menezes]

It's been committed as 228253.

Thank y'all for playing.

Cheers,

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: Kyrill Tkachov [mailto:kyrylo.tkachov@arm.com]
> Sent: Tuesday, September 29, 2015 4:01
> To: Marcus Shawcroft; Evandro Menezes; gcc-patches@gcc.gnu.org
> Cc: James Greenhalgh; Ramana Radhakrishnan
> Subject: Re: [PATCH][AArch64] Add separate insn sched class for vector LDP &
> STP
> 
> 
> On 29/09/15 09:03, Marcus Shawcroft wrote:
> > On 29/09/15 00:52, Evandro Menezes wrote:
> >> In some micro-architectures the insns to load or store pairs of
> >> vector registers are implemented rather differently from those
> >> affecting lanes in vector registers.  Then, it's important that such
> >> insns be described likewise differently in the scheduling model.
> >>
> >> This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart
> >> from the current neon_load2_2reg_q and neon_store2_2reg_q types,
> >> respectively.
> >>
> > Hi,
> >
> > The AArch64 part of this is OK. Please wait for Kyrill or Ramana to
> > comment on ARM side.  Cheers /Marcus
> >
> 
> This is ok arm-wise. I see the instructions being modelled with this type
> don't have a direct arm equivalent anyway.
> Marcus' comment on the ChangeLog still apply.
> 
> Thanks,
> Kyrill
> 
> >> Thank you,
> >>
> >> -- Evandro Menezes
> >>
> >>
> >> 0001-AArch64-Add-separate-insn-sched-class-for-vector-LDP.patch
> >>
> >>
> >>   From 340249dcd2af8dfce486cb4f62d4eaf285c6a799 Mon Sep 17 00:00:00
> >> 2001
> >> From: Evandro Menezes<e.menezes@samsung.com>
> >> Date: Mon, 28 Sep 2015 15:00:00 -0500
> >> Subject: [PATCH] [AArch64] Add separate insn sched class for vector
> >> LDP & STP
> >>
> >> 2015-09-28  Evandro Menezes<e.menezes@samsung.com>
> >>
> >> 	gcc/
> >> 	* config/arm/types.md (neon_ldp, neon_ldp_q, neon_stp, neon_stp_q):
> >> 	add new insn types for vector load and store pairs.
> > s/add/Add/ and likewise the rest of the changelog comments.
> >
> >> 	* config/arm/cortex-a53.md (cortex_a53_f_load_2reg): add insn
> >> 	types "neon_ldp{,_q}".
> >> 	* config/arm/cortex-a57.md (neon_load_c): add insn types
> >> 	"neon_ldp{,_q}".
> >> 	(neon_store_complex): add insn types "neon_stp{,_q}".
> >> 	* config/aarch64/aarch64-simd.md (aarch64_be_movoi): add insn types
> >> 	"neon_{ldp,stp}_q".

Re: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Ramana Radhakrishnan]

On Tue, Sep 29, 2015 at 12:52 AM, Evandro Menezes <e.menezes@samsung.com> wrote:
> In some micro-architectures the insns to load or store pairs of vector
> registers are implemented rather differently from those affecting lanes in
> vector registers.  Then, it's important that such insns be described
> likewise differently in the scheduling model.
>
> This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart from
> the current neon_load2_2reg_q and neon_store2_2reg_q types, respectively.

In such types.md restructuring, please handle these in *all* affected
scheduler descriptions, in this case thunder and xgene are 2 scheduler
descriptions that you have missed - Given Andrew is handling Thunder,
please update the xgene backend too at the same time. I can't think of
anything else that is affected right now.

A simple way to do that is to rename the old form to something else in
an intermediate patch using git to figure out all the
micro-architectures affected that need to be handled for both arm and
aarch64 backends and then add the new forms to handle this.

If there need to be follow up patches for xgene with different
handling, I'm sure Philipp will follow up - added him to CC.

Thanks,
Ramana

>
> Thank you,
>
> --
> Evandro Menezes
>

RE: [PATCH][AArch64] Add separate insn sched class for vector LDP & STP

[Evandro Menezes]

Hi, Rama.

My patch changed the type of a couple of A64 insns from "neon_load2_2reg_q" and "neon_store2_2reg_q".  However, neither ThunderX nor Xgene referred to these types, only A57 and A53 did.  So I didn't feel that I'd be the best person to add them to those machines.

Thank you,

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
> Behalf Of Ramana Radhakrishnan
> Sent: Tuesday, September 29, 2015 19:47
> To: Evandro Menezes
> Cc: gcc-patches; James Greenhalgh; Marcus Shawcroft; Philipp Tomsich
> Subject: Re: [PATCH][AArch64] Add separate insn sched class for vector LDP &
> STP
> 
> On Tue, Sep 29, 2015 at 12:52 AM, Evandro Menezes <e.menezes@samsung.com>
> wrote:
> > In some micro-architectures the insns to load or store pairs of vector
> > registers are implemented rather differently from those affecting
> > lanes in vector registers.  Then, it's important that such insns be
> > described likewise differently in the scheduling model.
> >
> > This patch adds the insn types neon_ldp{,_q} and neon_stp{,_q} apart
> > from the current neon_load2_2reg_q and neon_store2_2reg_q types,
> respectively.
> 
> In such types.md restructuring, please handle these in *all* affected
> scheduler descriptions, in this case thunder and xgene are 2 scheduler
> descriptions that you have missed - Given Andrew is handling Thunder, please
> update the xgene backend too at the same time. I can't think of anything else
> that is affected right now.
> 
> A simple way to do that is to rename the old form to something else in an
> intermediate patch using git to figure out all the micro-architectures
> affected that need to be handled for both arm and
> aarch64 backends and then add the new forms to handle this.
> 
> If there need to be follow up patches for xgene with different handling, I'm
> sure Philipp will follow up - added him to CC.
> 
> Thanks,
> Ramana
> 
> >
> > Thank you,
> >
> > --
> > Evandro Menezes
> >