[V3,2/4] RISC-V: Add vector related pipelines

Message ID 20240112180844.2005246-3-ewlu@rivosinc.com
State Unresolved
Headers
Series RISC-V: Associate typed insns to dfa reservation |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Edwin Lu Jan. 12, 2024, 6:08 p.m. UTC
  Creates new generic vector pipeline file common to all cpu tunes.
Moves all vector related pipelines from generic-ooo to generic-vector-ooo.
Creates new vector crypto related insn reservations. Add temporary attribute
for making changes to the vector cost model

gcc/ChangeLog:

	* config/riscv/generic-ooo.md (generic_ooo): Move reservation
	(generic_ooo_vec_load): ditto
	(generic_ooo_vec_store): ditto
	(generic_ooo_vec_loadstore_seg): ditto
	(generic_ooo_vec_alu): ditto
	(generic_ooo_vec_fcmp): ditto
	(generic_ooo_vec_imul): ditto
	(generic_ooo_vec_fadd): ditto
	(generic_ooo_vec_fmul): ditto
	(generic_ooo_crypto): ditto
	(generic_ooo_perm): ditto
	(generic_ooo_vec_reduction): ditto
	(generic_ooo_vec_ordered_reduction): ditto
	(generic_ooo_vec_idiv): ditto
	(generic_ooo_vec_float_divsqrt): ditto
	(generic_ooo_vec_mask): ditto
	(generic_ooo_vec_vesetvl): ditto
	(generic_ooo_vec_setrm): ditto
	(generic_ooo_vec_readlen): ditto
	* config/riscv/riscv.md (no): add temporary attribute
	* config/riscv/generic-vector-ooo.md: to here

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
Co-authored-by: Robin Dapp <rdapp.gcc@gmail.com>
---
V2:
- Remove unnecessary syntax changes in generic-ooo
- Add new vector crypto reservations and types to
  pipelines
V3:
- Move all vector pipelines into separate file which defines all ooo vector
  reservations.
- Add temporary attribute while cost model changes.
---
 gcc/config/riscv/generic-ooo.md        | 125 -------------------
 gcc/config/riscv/generic-vector-ooo.md | 165 +++++++++++++++++++++++++
 gcc/config/riscv/riscv.md              |   5 +
 3 files changed, 170 insertions(+), 125 deletions(-)
 create mode 100644 gcc/config/riscv/generic-vector-ooo.md
  

Comments

Robin Dapp Jan. 25, 2024, 5:06 p.m. UTC | #1
Thanks, that looks better IMHO.

> +;; Copyright (C) 2011-2024 Free Software Foundation, Inc.
> +;; Contributed by Andrew Waterman (andrew@sifive.com).
> +;; Based on MIPS target for GNU compiler.

You might want to change that, as well as the date.  While at
it you can also fix the broken date in my original file ;)

> +(define_insn_reservation "vec_load" 6
> +  (and (eq_attr "is_inorder" "no")
> +       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr"))
> +  "vxu_ooo_issue,vxu_ooo_alu")

I would rather ditch the is_inorder attribute for now and define
"low" latencies as well as reservations explicitly once we're
sure rather than falling back to scheduler defaults. 

OK with those changes.

Regards
 Robin
  
Edwin Lu Jan. 26, 2024, 6:25 p.m. UTC | #2
On 1/25/2024 9:06 AM, Robin Dapp wrote:
> Thanks, that looks better IMHO.
>
>> +;; Copyright (C) 2011-2024 Free Software Foundation, Inc.
>> +;; Contributed by Andrew Waterman (andrew@sifive.com).
>> +;; Based on MIPS target for GNU compiler.
> You might want to change that, as well as the date.  While at
> it you can also fix the broken date in my original file ;)


Completely forgot about this. I'll update it :)

>> +(define_insn_reservation "vec_load" 6
>> +  (and (eq_attr "is_inorder" "no")
>> +       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr"))
>> +  "vxu_ooo_issue,vxu_ooo_alu")
> I would rather ditch the is_inorder attribute for now and define
> "low" latencies as well as reservations explicitly once we're
> sure rather than falling back to scheduler defaults.


I think removing the is_inorder attribute should be ok. I added it 
because I wanted to avoid having two matching insn reservations defined 
since matching solely on the type attribute should also match on all 
subsets as well (i.e. if eventually we add an insn reservation checking 
for type "vlde" and tune "generic-ooo", any "vlde" insn would map to 
both reservations)

For now I should just remove the is_inorder attribute. We will update 
the latencies and add new reservations after we know what they should 
be. Is that correct?

> OK with those changes.
>
> Regards
>   Robin
  
Robin Dapp Jan. 30, 2024, 7:39 p.m. UTC | #3
> I think removing the is_inorder attribute should be ok. I added it
> because I wanted to avoid having two matching insn reservations
> defined since matching solely on the type attribute should also match
> on all subsets as well (i.e. if eventually we add an insn reservation
> checking for type "vlde" and tune "generic-ooo", any "vlde" insn
> would map to both reservations)
Ah, I see.  Yes we should prevent that from happening and in case we
have two (or more) similarly named reservations that would both match
such an attribute would make sense I guess.  My preference would just
be to not just add it yet before we know what else we'll be needing.
Chance is, not a whole lot will change until the release ;)

> For now I should just remove the is_inorder attribute. We will update
> the latencies and add new reservations after we know what they should
> be. Is that correct?

Yes, that should work.

Regards
 Robin
  

Patch

diff --git a/gcc/config/riscv/generic-ooo.md b/gcc/config/riscv/generic-ooo.md
index ef8cb96daf4..40e5104cde1 100644
--- a/gcc/config/riscv/generic-ooo.md
+++ b/gcc/config/riscv/generic-ooo.md
@@ -48,9 +48,6 @@  (define_automaton "generic_ooo")
 ;; Integer/float issue queues.
 (define_cpu_unit "issue0,issue1,issue2,issue3,issue4" "generic_ooo")
 
-;; Separate issue queue for vector instructions.
-(define_cpu_unit "generic_ooo_vxu_issue" "generic_ooo")
-
 ;; Integer/float execution units.
 (define_cpu_unit "ixu0,ixu1,ixu2,ixu3" "generic_ooo")
 (define_cpu_unit "fxu0,fxu1" "generic_ooo")
@@ -58,12 +55,6 @@  (define_cpu_unit "fxu0,fxu1" "generic_ooo")
 ;; Integer subunit for division.
 (define_cpu_unit "generic_ooo_div" "generic_ooo")
 
-;; Vector execution unit.
-(define_cpu_unit "generic_ooo_vxu_alu" "generic_ooo")
-
-;; Vector subunit that does mult/div/sqrt.
-(define_cpu_unit "generic_ooo_vxu_multicycle" "generic_ooo")
-
 ;; Shortcuts
 (define_reservation "generic_ooo_issue" "issue0|issue1|issue2|issue3|issue4")
 (define_reservation "generic_ooo_ixu_alu" "ixu0|ixu1|ixu2|ixu3")
@@ -92,25 +83,6 @@  (define_insn_reservation "generic_ooo_float_store" 6
        (eq_attr "type" "fpstore"))
   "generic_ooo_issue,generic_ooo_fxu")
 
-;; Vector load/store
-(define_insn_reservation "generic_ooo_vec_load" 6
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-(define_insn_reservation "generic_ooo_vec_store" 6
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vste,vstm,vsts,vstux,vstox,vstr"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector segment loads/stores.
-(define_insn_reservation "generic_ooo_vec_loadstore_seg" 10
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vlsegde,vlsegds,vlsegdux,vlsegdox,vlsegdff,\
-			vssegte,vssegts,vssegtux,vssegtox"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-
 ;; Generic integer instructions.
 (define_insn_reservation "generic_ooo_alu" 1
   (and (eq_attr "tune" "generic_ooo")
@@ -191,103 +163,6 @@  (define_insn_reservation "generic_ooo_popcount" 2
        (eq_attr "type" "cpop,clmul"))
   "generic_ooo_issue,generic_ooo_ixu_alu")
 
-;; Regular vector operations and integer comparisons.
-(define_insn_reservation "generic_ooo_vec_alu" 3
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vialu,viwalu,vext,vicalu,vshift,vnshift,viminmax,vicmp,\
-		        vimov,vsalu,vaalu,vsshift,vnclip,vmov,vfmov,vector"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector float comparison, conversion etc.
-(define_insn_reservation "generic_ooo_vec_fcmp" 3
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vfrecp,vfminmax,vfcmp,vfsgnj,vfclass,vfcvtitof,\
-			vfcvtftoi,vfwcvtitof,vfwcvtftoi,vfwcvtftof,vfncvtitof,\
-			vfncvtftoi,vfncvtftof"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector integer multiplication.
-(define_insn_reservation "generic_ooo_vec_imul" 4
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vimul,viwmul,vimuladd,viwmuladd,vsmul"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector float addition.
-(define_insn_reservation "generic_ooo_vec_fadd" 4
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vfalu,vfwalu"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector float multiplication and FMA.
-(define_insn_reservation "generic_ooo_vec_fmul" 6
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vfmul,vfwmul,vfmuladd,vfwmuladd"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector crypto, assumed to be a generic operation for now.
-(define_insn_reservation "generic_ooo_crypto" 4
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "crypto"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector permute.
-(define_insn_reservation "generic_ooo_perm" 3
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vimerge,vfmerge,vslideup,vslidedown,vislide1up,\
-			vislide1down,vfslide1up,vfslide1down,vgather,vcompress"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector reduction.
-(define_insn_reservation "generic_ooo_vec_reduction" 8
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vired,viwred,vfredu,vfwredu"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_multicycle")
-
-;; Vector ordered reduction, assume the latency number is for
-;; a 128-bit vector.  It is scaled in riscv_sched_adjust_cost
-;; for larger vectors.
-(define_insn_reservation "generic_ooo_vec_ordered_reduction" 10
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vfredo,vfwredo"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_multicycle*3")
-
-;; Vector integer division, assume not pipelined.
-(define_insn_reservation "generic_ooo_vec_idiv" 16
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vidiv"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_multicycle*3")
-
-;; Vector float divisions and sqrt, assume not pipelined.
-(define_insn_reservation "generic_ooo_vec_float_divsqrt" 16
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vfdiv,vfsqrt"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_multicycle*3")
-
-;; Vector mask operations.
-(define_insn_reservation "generic_ooo_vec_mask" 2
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vmalu,vmpop,vmffs,vmsfs,vmiota,vmidx,vimovvx,vimovxv,\
-			vfmovvf,vfmovfv"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
-
-;; Vector vsetvl.
-(define_insn_reservation "generic_ooo_vec_vesetvl" 1
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "vsetvl,vsetvl_pre"))
-  "generic_ooo_vxu_issue")
-
-;; Vector rounding mode setters, assume pipeline barrier.
-(define_insn_reservation "generic_ooo_vec_setrm" 20
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "wrvxrm,wrfrm"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_issue*3")
-
-;; Vector read vlen/vlenb.
-(define_insn_reservation "generic_ooo_vec_readlen" 4
-  (and (eq_attr "tune" "generic_ooo")
-       (eq_attr "type" "rdvlenb,rdvl"))
-  "generic_ooo_vxu_issue,generic_ooo_vxu_issue")
-
 ;; Transfer from/to coprocessor.  Assume not pipelined.
 (define_insn_reservation "generic_ooo_xfer" 4
   (and (eq_attr "tune" "generic_ooo")
diff --git a/gcc/config/riscv/generic-vector-ooo.md b/gcc/config/riscv/generic-vector-ooo.md
new file mode 100644
index 00000000000..474200ac26d
--- /dev/null
+++ b/gcc/config/riscv/generic-vector-ooo.md
@@ -0,0 +1,165 @@ 
+;; Copyright (C) 2011-2024 Free Software Foundation, Inc.
+;; Contributed by Andrew Waterman (andrew@sifive.com).
+;; Based on MIPS target for GNU compiler.
+
+;; This file is part of GCC.
+
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published
+;; by the Free Software Foundation; either version 3, or (at your
+;; option) any later version.
+
+;; GCC is distributed in the hope that it will be useful, but WITHOUT
+;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+;; or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+;; License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3.  If not see
+;; <http://www.gnu.org/licenses/>.
+;; Vector load/store
+
+(define_automaton "vector_ooo")
+
+;; Separate issue queue for vector instructions.
+(define_cpu_unit "vxu_ooo_issue" "vector_ooo")
+
+;; Vector execution unit.
+(define_cpu_unit "vxu_ooo_alu" "vector_ooo")
+
+;; Vector subunit that does mult/div/sqrt.
+(define_cpu_unit "vxu_ooo_multicycle" "vector_ooo")
+
+(define_insn_reservation "vec_load" 6
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+(define_insn_reservation "vec_store" 6
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vste,vstm,vsts,vstux,vstox,vstr"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector segment loads/stores.
+(define_insn_reservation "vec_loadstore_seg" 10
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vlsegde,vlsegds,vlsegdux,vlsegdox,vlsegdff,\
+		        vssegte,vssegts,vssegtux,vssegtox"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Regular vector operations and integer comparisons.
+(define_insn_reservation "vec_alu" 3
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vialu,viwalu,vext,vicalu,vshift,vnshift,viminmax,vicmp,\
+		        vimov,vsalu,vaalu,vsshift,vnclip,vmov,vfmov,vector,\
+		        vandn,vbrev,vbrev8,vrev8,vclz,vctz,vrol,vror,vwsll"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector float comparison, conversion etc.
+(define_insn_reservation "vec_fcmp" 3
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vfrecp,vfminmax,vfcmp,vfsgnj,vfclass,vfcvtitof,\
+		        vfcvtftoi,vfwcvtitof,vfwcvtftoi,vfwcvtftof,vfncvtitof,\
+		        vfncvtftoi,vfncvtftof"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector integer multiplication.
+(define_insn_reservation "vec_imul" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vimul,viwmul,vimuladd,viwmuladd,vsmul,vclmul,vclmulh,\
+		        vghsh,vgmul"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector float addition.
+(define_insn_reservation "vec_fadd" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vfalu,vfwalu"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector float multiplication and FMA.
+(define_insn_reservation "vec_fmul" 6
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vfmul,vfwmul,vfmuladd,vfwmuladd"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector crypto, assumed to be a generic operation for now.
+(define_insn_reservation "vec_crypto" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "crypto"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector crypto, AES
+(define_insn_reservation "vec_crypto_aes" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vaesef,vaesem,vaesdf,vaesdm,vaeskf1,vaeskf2,vaesz"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector crypto, sha
+(define_insn_reservation "vec_crypto_sha" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vsha2ms,vsha2ch,vsha2cl"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector crypto, SM3/4
+(define_insn_reservation "vec_crypto_sm" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vsm4k,vsm4r,vsm3me,vsm3c"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector permute.
+(define_insn_reservation "vec_perm" 3
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vimerge,vfmerge,vslideup,vslidedown,vislide1up,\
+		        vislide1down,vfslide1up,vfslide1down,vgather,vcompress"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector reduction.
+(define_insn_reservation "vec_reduction" 8
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vired,viwred,vfredu,vfwredu"))
+  "vxu_ooo_issue,vxu_ooo_multicycle")
+
+;; Vector ordered reduction, assume the latency number is for
+;; a 128-bit vector.  It is scaled in riscv_sched_adjust_cost
+;; for larger vectors.
+(define_insn_reservation "vec_ordered_reduction" 10
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vfredo,vfwredo"))
+  "vxu_ooo_issue,vxu_ooo_multicycle*3")
+
+;; Vector integer division, assume not pipelined.
+(define_insn_reservation "vec_idiv" 16
+  (eq_attr "type" "vidiv")
+  "vxu_ooo_issue,vxu_ooo_multicycle*3")
+
+;; Vector float divisions and sqrt, assume not pipelined.
+(define_insn_reservation "vec_float_divsqrt" 16
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vfdiv,vfsqrt"))
+  "vxu_ooo_issue,vxu_ooo_multicycle*3")
+
+;; Vector mask operations.
+(define_insn_reservation "vec_mask" 2
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vmalu,vmpop,vmffs,vmsfs,vmiota,vmidx,vimovvx,vimovxv,\
+		        vfmovvf,vfmovfv"))
+  "vxu_ooo_issue,vxu_ooo_alu")
+
+;; Vector vsetvl.
+(define_insn_reservation "vec_vesetvl" 1
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "vsetvl,vsetvl_pre"))
+  "vxu_ooo_issue")
+
+;; Vector rounding mode setters, assume pipeline barrier.
+(define_insn_reservation "vec_setrm" 20
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "wrvxrm,wrfrm"))
+  "vxu_ooo_issue,vxu_ooo_issue*3")
+
+;; Vector read vlen/vlenb.
+(define_insn_reservation "vec_readlen" 4
+  (and (eq_attr "is_inorder" "no")
+       (eq_attr "type" "rdvlenb,rdvl"))
+  "vxu_ooo_issue,vxu_ooo_issue")
+
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 1ec3e165791..a386d1aa694 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -664,6 +664,10 @@  (define_attr "tune"
   "generic,sifive_7,generic_ooo"
   (const (symbol_ref "((enum attr_tune) riscv_microarchitecture)")))
 
+;; In order/Out of order
+(define_attr "is_inorder" "no,yes" (const_string "no"))
+
+
 ;; Describe a user's asm statement.
 (define_asm_attributes
   [(set_attr "type" "multi")])
@@ -3827,6 +3831,7 @@  (define_insn "*large_load_address"
 (include "generic.md")
 (include "sifive-7.md")
 (include "thead.md")
+(include "generic-vector-ooo.md")
 (include "generic-ooo.md")
 (include "vector.md")
 (include "vector-crypto.md")