[PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation

[PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation

[Edwin Lu]

This series is a prototype for adding all typed instructions to a dfa 
scheduling pipeline.

I've been working on adding insn reservations for all typed instructions
to ensure all instructions are part of a dfa pipeline. I don't have a good 
understanding of vector instruction latency, so I have been struggling
with what I should do for those. 

As of right now, I have copied the insn reservations from generic-ooo.md 
for vector instructions into the generic.md and sifive-7.md files. This 
prevents ICEs from enabling the assert but introduces numerous scan
dump failures (when tested in linux rv64gcv and rv64gc_zba_zbb_zbc_zbs).

Currently, only patch 1/3 RISC-V: Add non-vector types to pipelines does
not introduce regressions (when tested against linux rv32/64 gc/gcv
on rocket).  I hope that the locations I added the insn types make sense. 
Please let me know if they should change.

The final patch enables the assert for insn_has_dfa_reservation. 

I tested the full patch series on both rocket and sifive-7-series. The 
series does introduce additional scan dump failures compared to their
respective baselines, however, I'm not sure how many failures were
due to the patch vs incorrect modeling assumptions. I created
PR113035 which has the full testsuite failures I saw (without the patches
applied).

Edwin Lu (3):
  RISC-V: Add non-vector types to pipelines
  RISC-V: Add vector related reservations
  RISC-V: Enable assert for insn_has_dfa_reservation

 gcc/config/riscv/generic-ooo.md |  31 ++++----
 gcc/config/riscv/generic.md     | 131 +++++++++++++++++++++++++++++++-
 gcc/config/riscv/riscv.cc       |   2 -
 gcc/config/riscv/sifive-7.md    | 130 ++++++++++++++++++++++++++++++-
 4 files changed, 271 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines

[Edwin Lu]

This patch does not create vector related insn reservations for
generic.md and sifive-7.md. It updates/creates insn reservations
for all non-vector typed insns

gcc/ChangeLog:

	* config/riscv/generic-ooo.md (generic_ooo_sfb_alu): create/update reservation
	(generic_ooo_branch): ditto
	* config/riscv/generic.md (generic_sfb_alu): ditto
	* config/riscv/sifive-7.md (sifive_7_popcount): ditto

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
 gcc/config/riscv/generic-ooo.md | 16 +++++++++++++---
 gcc/config/riscv/generic.md     | 13 +++++++++----
 gcc/config/riscv/sifive-7.md    | 12 +++++++++---
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/gcc/config/riscv/generic-ooo.md b/gcc/config/riscv/generic-ooo.md
index 78b9e48f935..de93245f965 100644
--- a/gcc/config/riscv/generic-ooo.md
+++ b/gcc/config/riscv/generic-ooo.md
@@ -95,7 +95,7 @@ (define_insn_reservation "generic_ooo_float_store" 6
 ;; 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"))
+       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
   "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
 
 (define_insn_reservation "generic_ooo_vec_store" 6
@@ -115,9 +115,19 @@ (define_insn_reservation "generic_ooo_vec_loadstore_seg" 10
 (define_insn_reservation "generic_ooo_alu" 1
   (and (eq_attr "tune" "generic_ooo")
        (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,\
-			move,bitmanip,min,max,minu,maxu,clz,ctz"))
+			move,bitmanip,rotate,min,max,minu,maxu,clz,ctz,atomic,condmove,cbo,mvpair,zicond"))
   "generic_ooo_issue,generic_ooo_ixu_alu")
 
+(define_insn_reservation "generic_ooo_sfb_alu" 2
+  (and (eq_attr "tune" "generic_ooo")
+       (eq_attr "type" "sfb_alu"))
+  "generic_ooo_issue,generic_ooo_ixu_alu")
+
+;; Branch instructions
+(define_insn_reservation "generic_ooo_branch" 1
+  (and (eq_attr "tune" "generic_ooo")
+       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
+  "generic_ooo_issue,generic_ooo_ixu_alu")
 
 ;; Float move, convert and compare.
 (define_insn_reservation "generic_ooo_float_move" 3
@@ -184,7 +194,7 @@ (define_insn_reservation "generic_ooo_popcount" 2
 (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"))
+		        vimov,vsalu,vaalu,vsshift,vnclip,vmov,vfmov,vector"))
   "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
 
 ;; Vector float comparison, conversion etc.
diff --git a/gcc/config/riscv/generic.md b/gcc/config/riscv/generic.md
index 88940483829..3e49d942495 100644
--- a/gcc/config/riscv/generic.md
+++ b/gcc/config/riscv/generic.md
@@ -27,7 +27,7 @@ (define_cpu_unit "fdivsqrt" "pipe0")
 
 (define_insn_reservation "generic_alu" 1
   (and (eq_attr "tune" "generic")
-       (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,move,bitmanip,min,max,minu,maxu,clz,ctz,cpop"))
+       (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,move,bitmanip,min,max,minu,maxu,clz,ctz,rotate,atomic,condmove,crypto,mvpair,zicond"))
   "alu")
 
 (define_insn_reservation "generic_load" 3
@@ -42,17 +42,22 @@ (define_insn_reservation "generic_store" 1
 
 (define_insn_reservation "generic_xfer" 3
   (and (eq_attr "tune" "generic")
-       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp"))
+       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp,cbo"))
   "alu")
 
 (define_insn_reservation "generic_branch" 1
   (and (eq_attr "tune" "generic")
-       (eq_attr "type" "branch,jump,call,jalr"))
+       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
+  "alu")
+
+(define_insn_reservation "generic_sfb_alu" 2
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "sfb_alu"))
   "alu")
 
 (define_insn_reservation "generic_imul" 10
   (and (eq_attr "tune" "generic")
-       (eq_attr "type" "imul,clmul"))
+       (eq_attr "type" "imul,clmul,cpop"))
   "imuldiv*10")
 
 (define_insn_reservation "generic_idivsi" 34
diff --git a/gcc/config/riscv/sifive-7.md b/gcc/config/riscv/sifive-7.md
index a63394c8c58..65d27cf6dc9 100644
--- a/gcc/config/riscv/sifive-7.md
+++ b/gcc/config/riscv/sifive-7.md
@@ -34,7 +34,7 @@ (define_insn_reservation "sifive_7_fpstore" 1
 
 (define_insn_reservation "sifive_7_branch" 1
   (and (eq_attr "tune" "sifive_7")
-       (eq_attr "type" "branch"))
+       (eq_attr "type" "branch,ret,trap"))
   "sifive_7_B")
 
 (define_insn_reservation "sifive_7_sfb_alu" 2
@@ -44,7 +44,7 @@ (define_insn_reservation "sifive_7_sfb_alu" 2
 
 (define_insn_reservation "sifive_7_jump" 1
   (and (eq_attr "tune" "sifive_7")
-       (eq_attr "type" "jump,call,jalr"))
+       (eq_attr "type" "jump,call,jalr,pushpop"))
   "sifive_7_B")
 
 (define_insn_reservation "sifive_7_mul" 3
@@ -59,7 +59,7 @@ (define_insn_reservation "sifive_7_div" 16
 
 (define_insn_reservation "sifive_7_alu" 2
   (and (eq_attr "tune" "sifive_7")
-       (eq_attr "type" "unknown,arith,shift,slt,multi,logical,move"))
+       (eq_attr "type" "unknown,arith,shift,slt,multi,logical,move,bitmanip,rotate,min,max,minu,maxu,clz,ctz,cbo,atomic,condmove,crypto,mvpair,zicond"))
   "sifive_7_A|sifive_7_B")
 
 (define_insn_reservation "sifive_7_load_immediate" 1
@@ -106,6 +106,12 @@ (define_insn_reservation "sifive_7_f2i" 3
        (eq_attr "type" "mfc"))
   "sifive_7_A")
 
+;; Popcount and clmul.
+(define_insn_reservation "sifive_7_popcount" 2
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "cpop,clmul"))
+  "sifive_7_A")
+
 (define_bypass 1 "sifive_7_load,sifive_7_alu,sifive_7_mul,sifive_7_f2i,sifive_7_sfb_alu"
   "sifive_7_alu,sifive_7_branch")
 
-- 
2.34.1

Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines

[Jeff Law]

On 12/15/23 11:53, Edwin Lu wrote:
> This patch does not create vector related insn reservations for
> generic.md and sifive-7.md. It updates/creates insn reservations
> for all non-vector typed insns
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/generic-ooo.md (generic_ooo_sfb_alu): create/update reservation
> 	(generic_ooo_branch): ditto
> 	* config/riscv/generic.md (generic_sfb_alu): ditto
> 	* config/riscv/sifive-7.md (sifive_7_popcount): ditto
> 
> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
> ---
>   gcc/config/riscv/generic-ooo.md | 16 +++++++++++++---
>   gcc/config/riscv/generic.md     | 13 +++++++++----
>   gcc/config/riscv/sifive-7.md    | 12 +++++++++---
>   3 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/gcc/config/riscv/generic-ooo.md b/gcc/config/riscv/generic-ooo.md
> index 78b9e48f935..de93245f965 100644
> --- a/gcc/config/riscv/generic-ooo.md
> +++ b/gcc/config/riscv/generic-ooo.md
> @@ -95,7 +95,7 @@ (define_insn_reservation "generic_ooo_float_store" 6
>   ;; 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"))
> +       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
>     "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
Hmm, I don't think "rdfrm" is really a vector load.  It's really a read 
of some bits in the fpcsr which elsewhere is handled as type fmove.  I'd 
actually suggest fixing vector.md to use fmove on the appropriate insn, 
then dropping rdfrm entirely.



>   
>   (define_insn_reservation "generic_xfer" 3
>     (and (eq_attr "tune" "generic")
> -       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp"))
> +       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp,cbo"))
>     "alu")
cbo is probably closer to a load/store than it is a transfer operation.

>   
>   (define_insn_reservation "generic_branch" 1
>     (and (eq_attr "tune" "generic")
> -       (eq_attr "type" "branch,jump,call,jalr"))
> +       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
> +  "alu")
pushpop are a mix of some pure memory operations and some mixed memory + 
branch.

However, from a scheduling standpoint the branch isn't particularly 
interesting.  So I'd have pushpop as a load/store.


jeff

Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines

[Edwin Lu]

On 12/20/2023 10:50 AM, Jeff Law wrote:
> 
> 
> On 12/15/23 11:53, Edwin Lu wrote:
>> This patch does not create vector related insn reservations for
>> generic.md and sifive-7.md. It updates/creates insn reservations
>> for all non-vector typed insns
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/generic-ooo.md (generic_ooo_sfb_alu): create/update 
>> reservation
>>     (generic_ooo_branch): ditto
>>     * config/riscv/generic.md (generic_sfb_alu): ditto
>>     * config/riscv/sifive-7.md (sifive_7_popcount): ditto
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> ---
>>   gcc/config/riscv/generic-ooo.md | 16 +++++++++++++---
>>   gcc/config/riscv/generic.md     | 13 +++++++++----
>>   gcc/config/riscv/sifive-7.md    | 12 +++++++++---
>>   3 files changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/gcc/config/riscv/generic-ooo.md 
>> b/gcc/config/riscv/generic-ooo.md
>> index 78b9e48f935..de93245f965 100644
>> --- a/gcc/config/riscv/generic-ooo.md
>> +++ b/gcc/config/riscv/generic-ooo.md
>> @@ -95,7 +95,7 @@ (define_insn_reservation "generic_ooo_float_store" 6
>>   ;; 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"))
>> +       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
>>     "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
> Hmm, I don't think "rdfrm" is really a vector load.  It's really a read 
> of some bits in the fpcsr which elsewhere is handled as type fmove.  I'd 
> actually suggest fixing vector.md to use fmove on the appropriate insn, 
> then dropping rdfrm entirely.
> Sounds good!
> 
> 
>>   (define_insn_reservation "generic_xfer" 3
>>     (and (eq_attr "tune" "generic")
>> -       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp"))
>> +       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp,cbo"))
>>     "alu")
> cbo is probably closer to a load/store than it is a transfer operation.
> 
That makes sense. I wasn't sure where exactly to put it since we have 
two separate insn reservations for load and store and from my 
understanding, the same type cannot be in two separate insn 
reservations. Would a new insn reservation like
(define_insn_reservation "generic_load_store" 2 ...) work?

>>   (define_insn_reservation "generic_branch" 1
>>     (and (eq_attr "tune" "generic")
>> -       (eq_attr "type" "branch,jump,call,jalr"))
>> +       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
>> +  "alu")
> pushpop are a mix of some pure memory operations and some mixed memory + 
> branch.
> 
> However, from a scheduling standpoint the branch isn't particularly 
> interesting.  So I'd have pushpop as a load/store.
> 
Same as above

Edwin

Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines

[Edwin Lu]

On 12/20/2023 10:50 AM, Jeff Law wrote:
> 
> 
> On 12/15/23 11:53, Edwin Lu wrote:
>> This patch does not create vector related insn reservations for
>> generic.md and sifive-7.md. It updates/creates insn reservations
>> for all non-vector typed insns
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/generic-ooo.md (generic_ooo_sfb_alu): create/update 
>> reservation
>>     (generic_ooo_branch): ditto
>>     * config/riscv/generic.md (generic_sfb_alu): ditto
>>     * config/riscv/sifive-7.md (sifive_7_popcount): ditto
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> ---
>>   gcc/config/riscv/generic-ooo.md | 16 +++++++++++++---
>>   gcc/config/riscv/generic.md     | 13 +++++++++----
>>   gcc/config/riscv/sifive-7.md    | 12 +++++++++---
>>   3 files changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/gcc/config/riscv/generic-ooo.md 
>> b/gcc/config/riscv/generic-ooo.md
>> index 78b9e48f935..de93245f965 100644
>> --- a/gcc/config/riscv/generic-ooo.md
>> +++ b/gcc/config/riscv/generic-ooo.md
>> @@ -95,7 +95,7 @@ (define_insn_reservation "generic_ooo_float_store" 6
>>   ;; 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"))
>> +       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
>>     "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
> Hmm, I don't think "rdfrm" is really a vector load.  It's really a read 
> of some bits in the fpcsr which elsewhere is handled as type fmove.  I'd 
> actually suggest fixing vector.md to use fmove on the appropriate insn, 
> then dropping rdfrm entirely.
> Sounds good!
> 
> 
>>   (define_insn_reservation "generic_xfer" 3
>>     (and (eq_attr "tune" "generic")
>> -       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp"))
>> +       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp,cbo"))
>>     "alu")
> cbo is probably closer to a load/store than it is a transfer operation.
> 
That makes sense. I wasn't sure where exactly to put it since we have 
two separate insn reservations for load and store and from my 
understanding, the same type cannot be in two separate insn 
reservations. Would a new insn reservation like
(define_insn_reservation "generic_load_store" 2 ...) work?

>>   (define_insn_reservation "generic_branch" 1
>>     (and (eq_attr "tune" "generic")
>> -       (eq_attr "type" "branch,jump,call,jalr"))
>> +       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
>> +  "alu")
> pushpop are a mix of some pure memory operations and some mixed memory + 
> branch.
> 
> However, from a scheduling standpoint the branch isn't particularly 
> interesting.  So I'd have pushpop as a load/store.
> 
Same as above

Edwin

Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines

[Jeff Law]

On 12/20/23 15:11, Edwin Lu wrote:

>>
>>
>>>   (define_insn_reservation "generic_xfer" 3
>>>     (and (eq_attr "tune" "generic")
>>> -       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp"))
>>> +       (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp,cbo"))
>>>     "alu")
>> cbo is probably closer to a load/store than it is a transfer operation.
>>
> That makes sense. I wasn't sure where exactly to put it since we have 
> two separate insn reservations for load and store and from my 
> understanding, the same type cannot be in two separate insn 
> reservations. Would a new insn reservation like
> (define_insn_reservation "generic_load_store" 2 ...) work?
I'd probably just treat cbo instructions as stores.  In fact, you could 
probably get away with using "store" as the type and dropping the cbo 
type entirely.




> 
>>>   (define_insn_reservation "generic_branch" 1
>>>     (and (eq_attr "tune" "generic")
>>> -       (eq_attr "type" "branch,jump,call,jalr"))
>>> +       (eq_attr "type" "branch,jump,call,jalr,ret,trap,pushpop"))
>>> +  "alu")
>> pushpop are a mix of some pure memory operations and some mixed memory 
>> + branch.
>>
>> However, from a scheduling standpoint the branch isn't particularly 
>> interesting.  So I'd have pushpop as a load/store.
So for these I think those which do pushes you could legitimately change 
  to the "store" type and pops could be changed to a "load" type.  If 
something does both just call it a load.  That's going to be the most 
useful from a scheduling standpoint I suspect.



Jeff

[PATCH 2/3][RFC] RISC-V: Add vector related reservations

[Edwin Lu]

This patch copies the vector reservations from generic-ooo.md and
inserts them into generic.md and sifive.md. The vector pipelines are
necessary to avoid an ICE from the assert

gcc/ChangeLog:

	* config/riscv/generic-ooo.md: syntax
	* config/riscv/generic.md (pipe0): new reservation
	(generic_vec_load): ditto
	(generic_vec_store): ditto
	(generic_vec_loadstore_seg): ditto
	(generic_generic_vec_alu): ditto
	(generic_vec_fcmp): ditto
	(generic_vec_imul): ditto
	(generic_vec_fadd): ditto
	(generic_vec_fmul): ditto
	(generic_crypto): ditto
	(generic_vec_perm): ditto
	(generic_vec_reduction): ditto
	(generic_vec_ordered_reduction): ditto
	(generic_vec_idiv): ditto
	(generic_vec_float_divsqrt): ditto
	(generic_vec_mask): ditto
	(generic_vec_vesetvl): ditto
	(generic_vec_setrm): ditto
	(generic_vec_readlen): ditto
	* config/riscv/sifive-7.md (sifive_7): new reservation
	(sifive_7_vec_load): ditto
	(sifive_7_vec_store): ditto
	(sifive_7_vec_loadstore_seg): ditto
	(sifive_7_sifive_7_vec_alu): ditto
	(sifive_7_vec_fcmp): ditto
	(sifive_7_vec_imul): ditto
	(sifive_7_vec_fadd): ditto
	(sifive_7_vec_fmul): ditto
	(sifive_7_crypto): ditto
	(sifive_7_vec_perm): ditto
	(sifive_7_vec_reduction): ditto
	(sifive_7_vec_ordered_reduction): ditto
	(sifive_7_vec_idiv): ditto
	(sifive_7_vec_float_divsqrt): ditto
	(sifive_7_vec_mask): ditto
	(sifive_7_vec_vesetvl): ditto
	(sifive_7_vec_setrm): ditto
	(sifive_7_vec_readlen): ditto

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
Co-authored-by: Robin Dapp <rdapp.gcc@gmail.com>
---
 gcc/config/riscv/generic-ooo.md |  19 ++---
 gcc/config/riscv/generic.md     | 118 ++++++++++++++++++++++++++++++++
 gcc/config/riscv/sifive-7.md    | 118 ++++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 13 deletions(-)

diff --git a/gcc/config/riscv/generic-ooo.md b/gcc/config/riscv/generic-ooo.md
index de93245f965..18b606bb981 100644
--- a/gcc/config/riscv/generic-ooo.md
+++ b/gcc/config/riscv/generic-ooo.md
@@ -106,16 +106,14 @@ (define_insn_reservation "generic_ooo_vec_store" 6
 ;; 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"))
+       (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")
-       (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,\
-			move,bitmanip,rotate,min,max,minu,maxu,clz,ctz,atomic,condmove,cbo,mvpair,zicond"))
+       (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,move,bitmanip,rotate,min,max,minu,maxu,clz,ctz,atomic,condmove,cbo,mvpair,zicond"))
   "generic_ooo_issue,generic_ooo_ixu_alu")
 
 (define_insn_reservation "generic_ooo_sfb_alu" 2
@@ -193,16 +191,13 @@ (define_insn_reservation "generic_ooo_popcount" 2
 ;; 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"))
+       (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"))
+       (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.
@@ -232,8 +227,7 @@ (define_insn_reservation "generic_ooo_crypto" 4
 ;; 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"))
+       (eq_attr "type" "vimerge,vfmerge,vslideup,vslidedown,vislide1up,vislide1down,vfslide1up,vfslide1down,vgather,vcompress"))
   "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
 
 ;; Vector reduction.
@@ -265,8 +259,7 @@ (define_insn_reservation "generic_ooo_vec_float_divsqrt" 16
 ;; 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"))
+       (eq_attr "type" "vmalu,vmpop,vmffs,vmsfs,vmiota,vmidx,vimovvx,vimovxv,vfmovvf,vfmovfv"))
   "generic_ooo_vxu_issue,generic_ooo_vxu_alu")
 
 ;; Vector vsetvl.
diff --git a/gcc/config/riscv/generic.md b/gcc/config/riscv/generic.md
index 3e49d942495..7ac974ad634 100644
--- a/gcc/config/riscv/generic.md
+++ b/gcc/config/riscv/generic.md
@@ -25,6 +25,15 @@ (define_cpu_unit "alu" "pipe0")
 (define_cpu_unit "imuldiv" "pipe0")
 (define_cpu_unit "fdivsqrt" "pipe0")
 
+;; Separate issue queue for vector instructions.
+(define_cpu_unit "generic_vec_issue" "pipe0")
+
+;; Vector alu unit
+(define_cpu_unit "generic_vec_alu" "pipe0")
+
+;; Vector mult/div/sqrt unit
+(define_cpu_unit "generic_vec_multi" "pipe0")
+
 (define_insn_reservation "generic_alu" 1
   (and (eq_attr "tune" "generic")
        (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,move,bitmanip,min,max,minu,maxu,clz,ctz,rotate,atomic,condmove,crypto,mvpair,zicond"))
@@ -93,3 +102,112 @@ (define_insn_reservation "generic_fsqrt" 25
   (and (eq_attr "tune" "generic")
        (eq_attr "type" "fsqrt"))
   "fdivsqrt*25")
+
+;; Vector load/store
+(define_insn_reservation "generic_vec_load" 6
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
+  "generic_vec_issue,generic_vec_alu")
+
+(define_insn_reservation "generic_vec_store" 6
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vste,vstm,vsts,vstux,vstox,vstr"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector segment loads/stores.
+(define_insn_reservation "generic_vec_loadstore_seg" 10
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vlsegde,vlsegds,vlsegdux,vlsegdox,vlsegdff,vssegte,vssegts,vssegtux,vssegtox"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Regular vector operations and integer comparisons.
+(define_insn_reservation "generic_generic_vec_alu" 3
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vialu,viwalu,vext,vicalu,vshift,vnshift,viminmax,vicmp,vimov,vsalu,vaalu,vsshift,vnclip,vmov,vfmov,vector"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector float comparison, conversion etc.
+(define_insn_reservation "generic_vec_fcmp" 3
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vfrecp,vfminmax,vfcmp,vfsgnj,vfclass,vfcvtitof,vfcvtftoi,vfwcvtitof,vfwcvtftoi,vfwcvtftof,vfncvtitof,vfncvtftoi,vfncvtftof"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector integer multiplication.
+(define_insn_reservation "generic_vec_imul" 4
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vimul,viwmul,vimuladd,viwmuladd,vsmul"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector float addition.
+(define_insn_reservation "generic_vec_fadd" 4
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vfalu,vfwalu"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector float multiplication and FMA.
+(define_insn_reservation "generic_vec_fmul" 6
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vfmul,vfwmul,vfmuladd,vfwmuladd"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector crypto, assumed to be a generic operation for now.
+(define_insn_reservation "generic_crypto" 4
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "crypto"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector permute.
+(define_insn_reservation "generic_vec_perm" 3
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vimerge,vfmerge,vslideup,vslidedown,vislide1up,vislide1down,vfslide1up,vfslide1down,vgather,vcompress"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector reduction.
+(define_insn_reservation "generic_vec_reduction" 8
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vired,viwred,vfredu,vfwredu"))
+  "generic_vec_issue,generic_vec_multi")
+
+;; 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_vec_ordered_reduction" 10
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vfredo,vfwredo"))
+  "generic_vec_issue,generic_vec_multi*3")
+
+;; Vector integer division, assume not pipelined.
+(define_insn_reservation "generic_vec_idiv" 16
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vidiv"))
+  "generic_vec_issue,generic_vec_multi*3")
+
+;; Vector float divisions and sqrt, assume not pipelined.
+(define_insn_reservation "generic_vec_float_divsqrt" 16
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vfdiv,vfsqrt"))
+  "generic_vec_issue,generic_vec_multi*3")
+
+;; Vector mask operations.
+(define_insn_reservation "generic_vec_mask" 2
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vmalu,vmpop,vmffs,vmsfs,vmiota,vmidx,vimovvx,vimovxv,vfmovvf,vfmovfv"))
+  "generic_vec_issue,generic_vec_alu")
+
+;; Vector vsetvl.
+(define_insn_reservation "generic_vec_vesetvl" 1
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "vsetvl,vsetvl_pre"))
+  "generic_vec_issue")
+
+;; Vector rounding mode setters, assume pipeline barrier.
+(define_insn_reservation "generic_vec_setrm" 20
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "wrvxrm,wrfrm"))
+  "generic_vec_issue,generic_vec_issue*3")
+
+;; Vector read vlen/vlenb.
+(define_insn_reservation "generic_vec_readlen" 4
+  (and (eq_attr "tune" "generic")
+       (eq_attr "type" "rdvlenb,rdvl"))
+  "generic_vec_issue,generic_vec_issue")
diff --git a/gcc/config/riscv/sifive-7.md b/gcc/config/riscv/sifive-7.md
index 65d27cf6dc9..bb71d1c5207 100644
--- a/gcc/config/riscv/sifive-7.md
+++ b/gcc/config/riscv/sifive-7.md
@@ -12,6 +12,15 @@ (define_cpu_unit "sifive_7_B" "sifive_7")
 (define_cpu_unit "sifive_7_idiv" "sifive_7")
 (define_cpu_unit "sifive_7_fpu" "sifive_7")
 
+;; Separate issue queue for vector instructions.
+(define_cpu_unit "sifive_7_vec_issue" "sifive_7")
+
+;; Vector alu unit
+(define_cpu_unit "sifive_7_vec_alu" "sifive_7")
+
+;; Vector mult/div/sqrt unit
+(define_cpu_unit "sifive_7_vec_multi" "sifive_7")
+
 (define_insn_reservation "sifive_7_load" 3
   (and (eq_attr "tune" "sifive_7")
        (eq_attr "type" "load"))
@@ -112,6 +121,115 @@ (define_insn_reservation "sifive_7_popcount" 2
        (eq_attr "type" "cpop,clmul"))
   "sifive_7_A")
 
+;; Vector load/store
+(define_insn_reservation "sifive_7_vec_load" 6
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vlde,vldm,vlds,vldux,vldox,vldff,vldr,rdfrm"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+(define_insn_reservation "sifive_7_vec_store" 6
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vste,vstm,vsts,vstux,vstox,vstr"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector segment loads/stores.
+(define_insn_reservation "sifive_7_vec_loadstore_seg" 10
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vlsegde,vlsegds,vlsegdux,vlsegdox,vlsegdff,vssegte,vssegts,vssegtux,vssegtox"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Regular vector operations and integer comparisons.
+(define_insn_reservation "sifive_7_sifive_7_vec_alu" 3
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vialu,viwalu,vext,vicalu,vshift,vnshift,viminmax,vicmp,vimov,vsalu,vaalu,vsshift,vnclip,vmov,vfmov,vector"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector float comparison, conversion etc.
+(define_insn_reservation "sifive_7_vec_fcmp" 3
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vfrecp,vfminmax,vfcmp,vfsgnj,vfclass,vfcvtitof,vfcvtftoi,vfwcvtitof,vfwcvtftoi,vfwcvtftof,vfncvtitof,vfncvtftoi,vfncvtftof"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector integer multiplication.
+(define_insn_reservation "sifive_7_vec_imul" 4
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vimul,viwmul,vimuladd,viwmuladd,vsmul"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector float addition.
+(define_insn_reservation "sifive_7_vec_fadd" 4
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vfalu,vfwalu"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector float multiplication and FMA.
+(define_insn_reservation "sifive_7_vec_fmul" 6
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vfmul,vfwmul,vfmuladd,vfwmuladd"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector crypto, assumed to be a generic operation for now.
+(define_insn_reservation "sifive_7_crypto" 4
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "crypto"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector permute.
+(define_insn_reservation "sifive_7_vec_perm" 3
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vimerge,vfmerge,vslideup,vslidedown,vislide1up,vislide1down,vfslide1up,vfslide1down,vgather,vcompress"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector reduction.
+(define_insn_reservation "sifive_7_vec_reduction" 8
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vired,viwred,vfredu,vfwredu"))
+  "sifive_7_vec_issue,sifive_7_vec_multi")
+
+;; 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 "sifive_7_vec_ordered_reduction" 10
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vfredo,vfwredo"))
+  "sifive_7_vec_issue,sifive_7_vec_multi*3")
+
+;; Vector integer division, assume not pipelined.
+(define_insn_reservation "sifive_7_vec_idiv" 16
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vidiv"))
+  "sifive_7_vec_issue,sifive_7_vec_multi*3")
+
+;; Vector float divisions and sqrt, assume not pipelined.
+(define_insn_reservation "sifive_7_vec_float_divsqrt" 16
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vfdiv,vfsqrt"))
+  "sifive_7_vec_issue,sifive_7_vec_multi*3")
+
+;; Vector mask operations.
+(define_insn_reservation "sifive_7_vec_mask" 2
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vmalu,vmpop,vmffs,vmsfs,vmiota,vmidx,vimovvx,vimovxv,vfmovvf,vfmovfv"))
+  "sifive_7_vec_issue,sifive_7_vec_alu")
+
+;; Vector vsetvl.
+(define_insn_reservation "sifive_7_vec_vesetvl" 1
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "vsetvl,vsetvl_pre"))
+  "sifive_7_vec_issue")
+
+;; Vector rounding mode setters, assume pipeline barrier.
+(define_insn_reservation "sifive_7_vec_setrm" 20
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "wrvxrm,wrfrm"))
+  "sifive_7_vec_issue,sifive_7_vec_issue*3")
+
+;; Vector read vlen/vlenb.
+(define_insn_reservation "sifive_7_vec_readlen" 4
+  (and (eq_attr "tune" "sifive_7")
+       (eq_attr "type" "rdvlenb,rdvl"))
+  "sifive_7_vec_issue,sifive_7_vec_issue")
+
 (define_bypass 1 "sifive_7_load,sifive_7_alu,sifive_7_mul,sifive_7_f2i,sifive_7_sfb_alu"
   "sifive_7_alu,sifive_7_branch")
 
-- 
2.34.1

Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations

[Jeff Law]

On 12/15/23 11:53, Edwin Lu wrote:
> This patch copies the vector reservations from generic-ooo.md and
> inserts them into generic.md and sifive.md. The vector pipelines are
> necessary to avoid an ICE from the assert

> 
> gcc/ChangeLog:
> 
> 	* config/riscv/generic-ooo.md: syntax
> 	* config/riscv/generic.md (pipe0): new reservation
> 	(generic_vec_load): ditto
> 	(generic_vec_store): ditto
> 	(generic_vec_loadstore_seg): ditto
> 	(generic_generic_vec_alu): ditto
> 	(generic_vec_fcmp): ditto
> 	(generic_vec_imul): ditto
> 	(generic_vec_fadd): ditto
> 	(generic_vec_fmul): ditto
> 	(generic_crypto): ditto
> 	(generic_vec_perm): ditto
> 	(generic_vec_reduction): ditto
> 	(generic_vec_ordered_reduction): ditto
> 	(generic_vec_idiv): ditto
> 	(generic_vec_float_divsqrt): ditto
> 	(generic_vec_mask): ditto
> 	(generic_vec_vesetvl): ditto
> 	(generic_vec_setrm): ditto
> 	(generic_vec_readlen): ditto
> 	* config/riscv/sifive-7.md (sifive_7): new reservation
> 	(sifive_7_vec_load): ditto
> 	(sifive_7_vec_store): ditto
> 	(sifive_7_vec_loadstore_seg): ditto
> 	(sifive_7_sifive_7_vec_alu): ditto
> 	(sifive_7_vec_fcmp): ditto
> 	(sifive_7_vec_imul): ditto
> 	(sifive_7_vec_fadd): ditto
> 	(sifive_7_vec_fmul): ditto
> 	(sifive_7_crypto): ditto
> 	(sifive_7_vec_perm): ditto
> 	(sifive_7_vec_reduction): ditto
> 	(sifive_7_vec_ordered_reduction): ditto
> 	(sifive_7_vec_idiv): ditto
> 	(sifive_7_vec_float_divsqrt): ditto
> 	(sifive_7_vec_mask): ditto
> 	(sifive_7_vec_vesetvl): ditto
> 	(sifive_7_vec_setrm): ditto
> 	(sifive_7_vec_readlen): ditto
> 
> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
> Co-authored-by: Robin Dapp <rdapp.gcc@gmail.com>
> ---
>   gcc/config/riscv/generic-ooo.md |  19 ++---
>   gcc/config/riscv/generic.md     | 118 ++++++++++++++++++++++++++++++++
>   gcc/config/riscv/sifive-7.md    | 118 ++++++++++++++++++++++++++++++++
>   3 files changed, 242 insertions(+), 13 deletions(-)
> 
> diff --git a/gcc/config/riscv/generic-ooo.md b/gcc/config/riscv/generic-ooo.md
> index de93245f965..18b606bb981 100644
> --- a/gcc/config/riscv/generic-ooo.md
> +++ b/gcc/config/riscv/generic-ooo.md
I'm not sure why you changed these.  In general we try to keep lines 
under 80 columns in the source.  Things like insn reservations are in a 
grey area as the lists sometimes get very long.  In general I'd leave 
this stuff alone if it doesn't have a function change.

>
> index 3e49d942495..7ac974ad634 100644
> --- a/gcc/config/riscv/generic.md
> +++ b/gcc/config/riscv/generic.md
Note that some of the stuff pointed out on patch #1 applies here to, 
like rdfrm not being a vector load/store.  So as you clean up patch #1, 
make sure to mirror the cleanups in patch #2 of the series.


Jeff

Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations

[Edwin Lu]

On 12/20/2023 10:57 AM, Jeff Law wrote:
> 
> 
> On 12/15/23 11:53, Edwin Lu wrote:
>> This patch copies the vector reservations from generic-ooo.md and
>> inserts them into generic.md and sifive.md. The vector pipelines are
>> necessary to avoid an ICE from the assert
> 
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/generic-ooo.md: syntax
>>     * config/riscv/generic.md (pipe0): new reservation
>>     (generic_vec_load): ditto
>>     (generic_vec_store): ditto
>>     (generic_vec_loadstore_seg): ditto
>>     (generic_generic_vec_alu): ditto
>>     (generic_vec_fcmp): ditto
>>     (generic_vec_imul): ditto
>>     (generic_vec_fadd): ditto
>>     (generic_vec_fmul): ditto
>>     (generic_crypto): ditto
>>     (generic_vec_perm): ditto
>>     (generic_vec_reduction): ditto
>>     (generic_vec_ordered_reduction): ditto
>>     (generic_vec_idiv): ditto
>>     (generic_vec_float_divsqrt): ditto
>>     (generic_vec_mask): ditto
>>     (generic_vec_vesetvl): ditto
>>     (generic_vec_setrm): ditto
>>     (generic_vec_readlen): ditto
>>     * config/riscv/sifive-7.md (sifive_7): new reservation
>>     (sifive_7_vec_load): ditto
>>     (sifive_7_vec_store): ditto
>>     (sifive_7_vec_loadstore_seg): ditto
>>     (sifive_7_sifive_7_vec_alu): ditto
>>     (sifive_7_vec_fcmp): ditto
>>     (sifive_7_vec_imul): ditto
>>     (sifive_7_vec_fadd): ditto
>>     (sifive_7_vec_fmul): ditto
>>     (sifive_7_crypto): ditto
>>     (sifive_7_vec_perm): ditto
>>     (sifive_7_vec_reduction): ditto
>>     (sifive_7_vec_ordered_reduction): ditto
>>     (sifive_7_vec_idiv): ditto
>>     (sifive_7_vec_float_divsqrt): ditto
>>     (sifive_7_vec_mask): ditto
>>     (sifive_7_vec_vesetvl): ditto
>>     (sifive_7_vec_setrm): ditto
>>     (sifive_7_vec_readlen): ditto
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> Co-authored-by: Robin Dapp <rdapp.gcc@gmail.com>
>> ---
>>   gcc/config/riscv/generic-ooo.md |  19 ++---
>>   gcc/config/riscv/generic.md     | 118 ++++++++++++++++++++++++++++++++
>>   gcc/config/riscv/sifive-7.md    | 118 ++++++++++++++++++++++++++++++++
>>   3 files changed, 242 insertions(+), 13 deletions(-)
>>
>> diff --git a/gcc/config/riscv/generic-ooo.md 
>> b/gcc/config/riscv/generic-ooo.md
>> index de93245f965..18b606bb981 100644
>> --- a/gcc/config/riscv/generic-ooo.md
>> +++ b/gcc/config/riscv/generic-ooo.md
> I'm not sure why you changed these.  In general we try to keep lines 
> under 80 columns in the source.  Things like insn reservations are in a 
> grey area as the lists sometimes get very long.  In general I'd leave 
> this stuff alone if it doesn't have a function change.
> 
Hmm when I was testing things before, I encountered an error where the 
scheduler had a problem with "\ <type>" with the assert enabled, but now 
I can't reproduce it. I'll revert it back to what it originally was and 
experiment some more.
>>
>> index 3e49d942495..7ac974ad634 100644
>> --- a/gcc/config/riscv/generic.md
>> +++ b/gcc/config/riscv/generic.md
> Note that some of the stuff pointed out on patch #1 applies here to, 
> like rdfrm not being a vector load/store.  So as you clean up patch #1, 
> make sure to mirror the cleanups in patch #2 of the series.
> 
Will do!

Edwin

Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations

[Edwin Lu]

On 12/20/2023 10:57 AM, Jeff Law wrote:
> 
> 
> On 12/15/23 11:53, Edwin Lu wrote:
>> This patch copies the vector reservations from generic-ooo.md and
>> inserts them into generic.md and sifive.md. The vector pipelines are
>> necessary to avoid an ICE from the assert
> 
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/generic-ooo.md: syntax
>>     * config/riscv/generic.md (pipe0): new reservation
>>     (generic_vec_load): ditto
>>     (generic_vec_store): ditto
>>     (generic_vec_loadstore_seg): ditto
>>     (generic_generic_vec_alu): ditto
>>     (generic_vec_fcmp): ditto
>>     (generic_vec_imul): ditto
>>     (generic_vec_fadd): ditto
>>     (generic_vec_fmul): ditto
>>     (generic_crypto): ditto
>>     (generic_vec_perm): ditto
>>     (generic_vec_reduction): ditto
>>     (generic_vec_ordered_reduction): ditto
>>     (generic_vec_idiv): ditto
>>     (generic_vec_float_divsqrt): ditto
>>     (generic_vec_mask): ditto
>>     (generic_vec_vesetvl): ditto
>>     (generic_vec_setrm): ditto
>>     (generic_vec_readlen): ditto
>>     * config/riscv/sifive-7.md (sifive_7): new reservation
>>     (sifive_7_vec_load): ditto
>>     (sifive_7_vec_store): ditto
>>     (sifive_7_vec_loadstore_seg): ditto
>>     (sifive_7_sifive_7_vec_alu): ditto
>>     (sifive_7_vec_fcmp): ditto
>>     (sifive_7_vec_imul): ditto
>>     (sifive_7_vec_fadd): ditto
>>     (sifive_7_vec_fmul): ditto
>>     (sifive_7_crypto): ditto
>>     (sifive_7_vec_perm): ditto
>>     (sifive_7_vec_reduction): ditto
>>     (sifive_7_vec_ordered_reduction): ditto
>>     (sifive_7_vec_idiv): ditto
>>     (sifive_7_vec_float_divsqrt): ditto
>>     (sifive_7_vec_mask): ditto
>>     (sifive_7_vec_vesetvl): ditto
>>     (sifive_7_vec_setrm): ditto
>>     (sifive_7_vec_readlen): ditto
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> Co-authored-by: Robin Dapp <rdapp.gcc@gmail.com>
>> ---
>>   gcc/config/riscv/generic-ooo.md |  19 ++---
>>   gcc/config/riscv/generic.md     | 118 ++++++++++++++++++++++++++++++++
>>   gcc/config/riscv/sifive-7.md    | 118 ++++++++++++++++++++++++++++++++
>>   3 files changed, 242 insertions(+), 13 deletions(-)
>>
>> diff --git a/gcc/config/riscv/generic-ooo.md 
>> b/gcc/config/riscv/generic-ooo.md
>> index de93245f965..18b606bb981 100644
>> --- a/gcc/config/riscv/generic-ooo.md
>> +++ b/gcc/config/riscv/generic-ooo.md
> I'm not sure why you changed these.  In general we try to keep lines 
> under 80 columns in the source.  Things like insn reservations are in a 
> grey area as the lists sometimes get very long.  In general I'd leave 
> this stuff alone if it doesn't have a function change.
> 
Hmm when I was testing things before, I encountered an error where the 
scheduler had a problem with "\ <type>" with the assert enabled, but now 
I can't reproduce it. I'll revert it back to what it originally was and 
experiment some more.
>>
>> index 3e49d942495..7ac974ad634 100644
>> --- a/gcc/config/riscv/generic.md
>> +++ b/gcc/config/riscv/generic.md
> Note that some of the stuff pointed out on patch #1 applies here to, 
> like rdfrm not being a vector load/store.  So as you clean up patch #1, 
> make sure to mirror the cleanups in patch #2 of the series.
> 
Will do!

Edwin

Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations

[Edwin Lu]

On 12/20/2023 2:55 PM, Edwin Lu wrote:
> On 12/20/2023 10:57 AM, Jeff Law wrote:
>>
>>
>> On 12/15/23 11:53, Edwin Lu wrote:
>>> This patch copies the vector reservations from generic-ooo.md and
>>> inserts them into generic.md and sifive.md. The vector pipelines are
>>> necessary to avoid an ICE from the assert

I forgot to get clarification earlier but this patch would introduce 
many scan-dump failures for both vector and non-vector targets 
(https://github.com/ewlu/gcc-precommit-ci/issues/950#issuecomment-1858392181). 
I haven't identified any execution errors that would be introduced on 
mtune=rocket aside from one ICE which I'm currently working on fixing.

Additionally, as mentioned in PR113035, there are significant testsuite 
differences between mtune=rocket and mtune=sifive-7-series. I haven't 
gone through all of the differences and I don't know if they are a 
problem with the patch or a result of the cost modeling assumptions.

Is there a problem with the current way mtune=rocket is modeled with 
generic.md?

Edwin

[PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation

[Edwin Lu]

Enables assert that every typed instruction is associated with a
dfa reservation

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_sched_variable_issue): enable assert

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
 gcc/config/riscv/riscv.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index ab0f95e5fe9..3adeb415bec 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8048,9 +8048,7 @@ riscv_sched_variable_issue (FILE *, int, rtx_insn *insn, int more)
 
   /* If we ever encounter an insn without an insn reservation, trip
      an assert so we can find and fix this problem.  */
-#if 0
   gcc_assert (insn_has_dfa_reservation_p (insn));
-#endif
 
   return more - 1;
 }
-- 
2.34.1

Re: [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation

[Jeff Law]

On 12/15/23 11:53, Edwin Lu wrote:
> Enables assert that every typed instruction is associated with a
> dfa reservation
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (riscv_sched_variable_issue): enable assert
Once the prereqs are in, this is fine.
jeff