diff mbox series

[aarch64] Add missing thunderx2-t99 instruction scheduling pipeline descriptions.

Message ID 1512153744.14842.3.camel@cavium.com
State New
Headers show
Series [aarch64] Add missing thunderx2-t99 instruction scheduling pipeline descriptions. | expand

Commit Message

Steve Ellcey Dec. 1, 2017, 6:42 p.m. UTC 
There are a number of instruction types defined in aarch64.md which do not
have pipeline/scheduling information in thunderx2-t99.md.  This patch adds
some of them.  This patch includes all the missing types except the neon
ones that I hope to include in a follow-up patch.

Bootstrapped and tested with no regressions on a thunderx2.

I know we are in stage3 but I hope this type of plaform specific
change is still OK to checkin.

Steve Ellcey
sellcey@cavium.com


2017-11-30  Steve Ellcey  <sellcey@cavium.com>

	* config/aarch64/thunderx2-t99.md (thunderx2t99_branch): Add trap
	to reservation.
	(thunderx2t99_nothing): New insn reservation.
	(thunderx2t99_mrs): New insn reservation.
	(thunderx2t99_multiple): New insn reservation.
	(thunderx2t99_alu_basi): Add bfx to reservation.
	(thunderx2t99_fp_cmp): Add fccmps and fccmpd to reservation.

Comments

Kyrill Tkachov Dec. 4, 2017, 5:18 p.m. UTC | #1 
Hi Steve,

On 01/12/17 18:42, Steve Ellcey wrote:
> There are a number of instruction types defined in aarch64.md which do not
> have pipeline/scheduling information in thunderx2-t99.md.  This patch adds
> some of them.  This patch includes all the missing types except the neon
> ones that I hope to include in a follow-up patch.
>
> Bootstrapped and tested with no regressions on a thunderx2.
>
> I know we are in stage3 but I hope this type of plaform specific
> change is still OK to checkin.
>

This looks ok to me (though I cannot approve) with a style nit inline.
I'd say this fairly low risk to take in, but it's up to the maintainers 
to make
the final call.

Thanks,
Kyrill

> Steve Ellcey
> sellcey@cavium.com
>
>
> 2017-11-30  Steve Ellcey  <sellcey@cavium.com>
>
>         * config/aarch64/thunderx2-t99.md (thunderx2t99_branch): Add trap
>         to reservation.
>         (thunderx2t99_nothing): New insn reservation.
>         (thunderx2t99_mrs): New insn reservation.
>         (thunderx2t99_multiple): New insn reservation.
>         (thunderx2t99_alu_basi): Add bfx to reservation.
>         (thunderx2t99_fp_cmp): Add fccmps and fccmpd to reservation.
>
>
> diff --git a/gcc/config/aarch64/thunderx2t99.md 
> b/gcc/config/aarch64/thunderx2t99.md
> index 5bcf4ff..5e48521 100644
> --- a/gcc/config/aarch64/thunderx2t99.md
> +++ b/gcc/config/aarch64/thunderx2t99.md
> @@ -69,9 +69,26 @@
>
>  (define_insn_reservation "thunderx2t99_branch" 1
>    (and (eq_attr "tune" "thunderx2t99")
> -       (eq_attr "type" "call,branch"))
> +       (eq_attr "type" "call,branch,trap"))
>    "thunderx2t99_i2")
>
> +;; Misc instructions.
> +
> +(define_insn_reservation "thunderx2t99_nothing" 0
> +  (and (eq_attr "tune" "thunderx2t99")
> +       (eq_attr "type" "no_insn,block"))
> +  "nothing")
> +
> +(define_insn_reservation "thunderx2t99_mrs" 0
> +  (and (eq_attr "tune" "thunderx2t99")
> +       (eq_attr "type" "mrs"))
> +  "thunderx2t99_i2")
> +
> +(define_insn_reservation "thunderx2t99_multiple" 1
> +  (and (eq_attr "tune" "thunderx2t99")
> +       (eq_attr "type" "multiple"))
> +  "thunderx2t99_i0+thunderx2t99_i1+thunderx2t99_i2+thunderx2t99_ls0+thunderx2t99_ls1+thunderx2t99_sd+thunderx2t99_i1m1+thunderx2t99_i1m2+thunderx2t99_i1m3+thunderx2t99_ls0d1+thunderx2t99_ls0d2+thunderx2t99_ls0d3+thunderx2t99_ls1d1+thunderx2t99_ls1d2+thunderx2t99_ls1d3+thunderx2t99_f0+thunderx2t99_f1")
> +

We try to adhere to the 80 columns per line rule in the scheduling 
description files as well,
so can you please use "\" to break this into multiple lines.

>  ;; Integer arithmetic/logic instructions.
>
>  ; Plain register moves are handled by renaming, and don't create any 
> uops.
> @@ -87,7 +104,7 @@
>                         adc_reg,adc_imm,adcs_reg,adcs_imm,\
> logic_reg,logic_imm,logics_reg,logics_imm,\
> csel,adr,mov_imm,shift_reg,shift_imm,bfm,\
> -                       rbit,rev,extend,rotate_imm"))
> +                       bfx,rbit,rev,extend,rotate_imm"))
>    "thunderx2t99_i012")
>
>  (define_insn_reservation "thunderx2t99_alu_shift" 2
> @@ -155,7 +172,7 @@
>
>  (define_insn_reservation "thunderx2t99_fp_cmp" 5
>    (and (eq_attr "tune" "thunderx2t99")
> -       (eq_attr "type" "fcmps,fcmpd"))
> +       (eq_attr "type" "fcmps,fcmpd,fccmps,fccmpd"))
>    "thunderx2t99_f01")
>
>  (define_insn_reservation "thunderx2t99_fp_divsqrt_s" 16
Steve Ellcey Dec. 4, 2017, 5:33 p.m. UTC | #2 
On Mon, 2017-12-04 at 17:18 +0000, Kyrill Tkachov wrote:

> > +(define_insn_reservation "thunderx2t99_multiple" 1
> > +  (and (eq_attr "tune" "thunderx2t99")
> > +       (eq_attr "type" "multiple"))
> > +  "thunderx2t99_i0+thunderx2t99_i1+thunderx2t99_i2+thunderx2t99_ls
> > 0+thunderx2t99_ls1+thunderx2t99_sd+thunderx2t99_i1m1+thunderx2t99_i
> > 1m2+thunderx2t99_i1m3+thunderx2t99_ls0d1+thunderx2t99_ls0d2+thunder
> > x2t99_ls0d3+thunderx2t99_ls1d1+thunderx2t99_ls1d2+thunderx2t99_ls1d
> > 3+thunderx2t99_f0+thunderx2t99_f1")
> > +
> We try to adhere to the 80 columns per line rule in the scheduling 
> description files as well,
> so can you please use "\" to break this into multiple lines.

Yes, I wasn't sure if whatever program parses this file would honor
backslashes so I didn't break it up.  The falkor_extra definition in
falkor.md that I looked at is more than 80 characters and that is one
of the reasons I wasn't sure backslashes would be honored.  But I see
other places (power8.md) where the backslashes are used so I will make
that change if and when the patch is approved.

Steve Ellcey
sellcey@cavium.com
James Greenhalgh Dec. 5, 2017, 2:39 p.m. UTC | #3 
On Mon, Dec 04, 2017 at 05:33:37PM +0000, Steve Ellcey wrote:
> On Mon, 2017-12-04 at 17:18 +0000, Kyrill Tkachov wrote:
> 
> > > +(define_insn_reservation "thunderx2t99_multiple" 1
> > > +  (and (eq_attr "tune" "thunderx2t99")
> > > +       (eq_attr "type" "multiple"))
> > > +  "thunderx2t99_i0+thunderx2t99_i1+thunderx2t99_i2+thunderx2t99_ls
> > > 0+thunderx2t99_ls1+thunderx2t99_sd+thunderx2t99_i1m1+thunderx2t99_i
> > > 1m2+thunderx2t99_i1m3+thunderx2t99_ls0d1+thunderx2t99_ls0d2+thunder
> > > x2t99_ls0d3+thunderx2t99_ls1d1+thunderx2t99_ls1d2+thunderx2t99_ls1d
> > > 3+thunderx2t99_f0+thunderx2t99_f1")
> > > +
> > We try to adhere to the 80 columns per line rule in the scheduling 
> > description files as well,
> > so can you please use "\" to break this into multiple lines.
> 
> Yes, I wasn't sure if whatever program parses this file would honor
> backslashes so I didn't break it up.  The falkor_extra definition in
> falkor.md that I looked at is more than 80 characters and that is one
> of the reasons I wasn't sure backslashes would be honored.  But I see
> other places (power8.md) where the backslashes are used so I will make
> that change if and when the patch is approved.

As long as it bootstraps I'm happy to take it - I'm sure you're better placed
to spot and understand regressions in thunderx2t99 performance than I am.

OK

Thanks,
James
diff mbox series

Patch

diff --git a/gcc/config/aarch64/thunderx2t99.md b/gcc/config/aarch64/thunderx2t99.md
index 5bcf4ff..5e48521 100644
--- a/gcc/config/aarch64/thunderx2t99.md
+++ b/gcc/config/aarch64/thunderx2t99.md
@@ -69,9 +69,26 @@ 
 
 (define_insn_reservation "thunderx2t99_branch" 1
   (and (eq_attr "tune" "thunderx2t99")
-       (eq_attr "type" "call,branch"))
+       (eq_attr "type" "call,branch,trap"))
   "thunderx2t99_i2")
 
+;; Misc instructions.
+
+(define_insn_reservation "thunderx2t99_nothing" 0
+  (and (eq_attr "tune" "thunderx2t99")
+       (eq_attr "type" "no_insn,block"))
+  "nothing")
+
+(define_insn_reservation "thunderx2t99_mrs" 0
+  (and (eq_attr "tune" "thunderx2t99")
+       (eq_attr "type" "mrs"))
+  "thunderx2t99_i2")
+
+(define_insn_reservation "thunderx2t99_multiple" 1
+  (and (eq_attr "tune" "thunderx2t99")
+       (eq_attr "type" "multiple"))
+  "thunderx2t99_i0+thunderx2t99_i1+thunderx2t99_i2+thunderx2t99_ls0+thunderx2t99_ls1+thunderx2t99_sd+thunderx2t99_i1m1+thunderx2t99_i1m2+thunderx2t99_i1m3+thunderx2t99_ls0d1+thunderx2t99_ls0d2+thunderx2t99_ls0d3+thunderx2t99_ls1d1+thunderx2t99_ls1d2+thunderx2t99_ls1d3+thunderx2t99_f0+thunderx2t99_f1")
+
 ;; Integer arithmetic/logic instructions.
 
 ; Plain register moves are handled by renaming, and don't create any uops.
@@ -87,7 +104,7 @@ 
 			adc_reg,adc_imm,adcs_reg,adcs_imm,\
 			logic_reg,logic_imm,logics_reg,logics_imm,\
 			csel,adr,mov_imm,shift_reg,shift_imm,bfm,\
-			rbit,rev,extend,rotate_imm"))
+			bfx,rbit,rev,extend,rotate_imm"))
   "thunderx2t99_i012")
 
 (define_insn_reservation "thunderx2t99_alu_shift" 2
@@ -155,7 +172,7 @@ 
 
 (define_insn_reservation "thunderx2t99_fp_cmp" 5
   (and (eq_attr "tune" "thunderx2t99")
-       (eq_attr "type" "fcmps,fcmpd"))
+       (eq_attr "type" "fcmps,fcmpd,fccmps,fccmpd"))
   "thunderx2t99_f01")
 
 (define_insn_reservation "thunderx2t99_fp_divsqrt_s" 16