[aarch64,v2] Fix falkor pipeline description for dup<q>

Message ID 20180802165837.23379-1-siddhesh@sourceware.org
State New
Headers show
Series
  • [aarch64,v2] Fix falkor pipeline description for dup<q>
Related show

Commit Message

Siddhesh Poyarekar Aug. 2, 2018, 4:58 p.m.
There was a typo in the pipeline description where DUP was assigned to
the vector pipes for quad mode ops when it really only uses the VTOG
pipes.  Fixing this does not show any noticeable difference in
performance (there's a very small bump of 1.7% in x264 but that's
about it) in my tests but is the more precise description of operations
for falkor.

Bootstrapped and tested with --with-cpu=falkor to confirm that there
are no regressions.

Siddhesh

Changes from v1:

- Fixed up ChangeLog entry

- Replaced falkor_gtov,falkor_gtov with falkor_gtov*2

	* config/aarch64/falkor.md (falkor_am_1_vxvy_vxvy): Move
	neon_dup_q to...
	(falkor_am_1_gtov_gtov): ... a new insn reservation.

CC: james.greenhalgh@arm.com
CC: kyrylo.tkachov@foss.arm.com
---
 gcc/config/aarch64/falkor.md | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

James Greenhalgh Aug. 2, 2018, 6:32 p.m. | #1
On Thu, Aug 02, 2018 at 11:58:37AM -0500, Siddhesh Poyarekar wrote:
> There was a typo in the pipeline description where DUP was assigned to
> the vector pipes for quad mode ops when it really only uses the VTOG
> pipes.  Fixing this does not show any noticeable difference in
> performance (there's a very small bump of 1.7% in x264 but that's
> about it) in my tests but is the more precise description of operations
> for falkor.
> 
> Bootstrapped and tested with --with-cpu=falkor to confirm that there
> are no regressions.

OK.

Thanks,
James

> 
> Siddhesh
> 
> Changes from v1:
> 
> - Fixed up ChangeLog entry
> 
> - Replaced falkor_gtov,falkor_gtov with falkor_gtov*2
> 
> 	* config/aarch64/falkor.md (falkor_am_1_vxvy_vxvy): Move
> 	neon_dup_q to...
> 	(falkor_am_1_gtov_gtov): ... a new insn reservation.
>
Siddhesh Poyarekar Aug. 3, 2018, 2:02 a.m. | #2
On 08/03/2018 12:02 AM, James Greenhalgh wrote:
> On Thu, Aug 02, 2018 at 11:58:37AM -0500, Siddhesh Poyarekar wrote:
>> There was a typo in the pipeline description where DUP was assigned to
>> the vector pipes for quad mode ops when it really only uses the VTOG
>> pipes.  Fixing this does not show any noticeable difference in
>> performance (there's a very small bump of 1.7% in x264 but that's
>> about it) in my tests but is the more precise description of operations
>> for falkor.
>>
>> Bootstrapped and tested with --with-cpu=falkor to confirm that there
>> are no regressions.
> 
> OK.

Thanks, may I backport this to gcc 8.x as well given that it has zero 
impact on anything except falkor?  We are targeting gcc 8.x as the 
toolchain for falkor and so would like to at least get all falkor 
specific optimizations and fixes back into the gcc 8.x branch.

Siddhesh
James Greenhalgh Aug. 7, 2018, 4:55 p.m. | #3
On Thu, Aug 02, 2018 at 09:02:05PM -0500, Siddhesh Poyarekar wrote:
> On 08/03/2018 12:02 AM, James Greenhalgh wrote:
> > On Thu, Aug 02, 2018 at 11:58:37AM -0500, Siddhesh Poyarekar wrote:
> >> There was a typo in the pipeline description where DUP was assigned to
> >> the vector pipes for quad mode ops when it really only uses the VTOG
> >> pipes.  Fixing this does not show any noticeable difference in
> >> performance (there's a very small bump of 1.7% in x264 but that's
> >> about it) in my tests but is the more precise description of operations
> >> for falkor.
> >>
> >> Bootstrapped and tested with --with-cpu=falkor to confirm that there
> >> are no regressions.
> > 
> > OK.
> 
> Thanks, may I backport this to gcc 8.x as well given that it has zero 
> impact on anything except falkor?  We are targeting gcc 8.x as the 
> toolchain for falkor and so would like to at least get all falkor 
> specific optimizations and fixes back into the gcc 8.x branch.

This one is OK for backport.

Thanks,
James

Patch

diff --git a/gcc/config/aarch64/falkor.md b/gcc/config/aarch64/falkor.md
index 5cbc5150ef3..45cbff93b24 100644
--- a/gcc/config/aarch64/falkor.md
+++ b/gcc/config/aarch64/falkor.md
@@ -322,6 +322,12 @@ 
        (eq_attr "type" "neon_from_gp_q"))
   "falkor_gtov,falkor_gtov")
 
+;; DUP  does not use vector pipes in Q mode, only gtov+gtov.
+(define_insn_reservation "falkor_am_1_gtov_gtov" 1
+  (and (eq_attr "tune" "falkor")
+       (eq_attr "type" "neon_dup_q"))
+  "falkor_gtov*2")
+
 ;; neon_to_gp_q is used for 32-bit ARM instructions that move 64-bits of data
 ;; so no use needed here.
 
@@ -337,7 +343,7 @@ 
 
 (define_insn_reservation "falkor_am_1_vxvy_vxvy" 1
   (and (eq_attr "tune" "falkor")
-       (eq_attr "type" "neon_bsl_q,neon_dup_q,neon_ext_q,neon_move_q,neon_rev_q,neon_tbl1_q,neon_permute_q"))
+       (eq_attr "type" "neon_bsl_q,neon_ext_q,neon_move_q,neon_rev_q,neon_tbl1_q,neon_permute_q"))
   "falkor_vxvy+falkor_vxvy")
 
 (define_insn_reservation "falkor_am_2_vxvy" 2