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

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.

	* gcc/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(-)

Hi Siddhesh,

On 02/08/18 11:23, 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.
>

You know the microarchitecture better, so I'm assuming that's correct for Falkor.
I assume this has been tested with a bootstrap and test.

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

No gcc/ prefix in the ChangeLog path.

> CC: james.greenhalgh@arm.com
> CC: kyrylo.tkachov@foss.arm.com
> ---
>  gcc/config/aarch64/falkor.md | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/aarch64/falkor.md b/gcc/config/aarch64/falkor.md
> index 5cbc5150ef3..83b4b14b7f4 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,falkor_gtov")

You should be able to use the "*" construct here like this:
falkor_gtov*2

Otherwise looks ok to me (but you'll need a maintainer to approve)
Thanks,
Kyrill

> +
>  ;; 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
> -- 
> 2.17.1
>

On 08/02/2018 04:19 PM, Kyrill Tkachov wrote:
> Hi Siddhesh,
> 
> On 02/08/18 11:23, 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.
>>
> 
> You know the microarchitecture better, so I'm assuming that's correct 
> for Falkor.
> I assume this has been tested with a bootstrap and test.

Sorry I should have mentioned this: bootstrapped and tested 
--with-cpu=falkor and no new regressions resulted from this patch.

> 
>>         * gcc/config/aarch64/falkor.md (falkor_am_1_vxvy_vxvy): Move
>>         neon_dup_q to...
>>         (falkor_am_1_gtov_gtov): ... a new insn reservation.
>>
> 
> No gcc/ prefix in the ChangeLog path.

Oops, will fix.

> You should be able to use the "*" construct here like this:
> falkor_gtov*2

Fixed and tested.

> Otherwise looks ok to me (but you'll need a maintainer to approve)

Thanks, I'll send the updated patch for James' review.

Siddhesh