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

[Robin Dapp]

> 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

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

[Edwin Lu]

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 (and...@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

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

[Robin Dapp]

Thanks, that looks better IMHO.

> +;; Copyright (C) 2011-2024 Free Software Foundation, Inc.
> +;; Contributed by Andrew Waterman (and...@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

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

[Edwin Lu]

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 
Co-authored-by: Robin Dapp 
---
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

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"