* [PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation
@ 2023-12-15 18:53 Edwin Lu
2023-12-15 18:53 ` [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines Edwin Lu
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-15 18:53 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng, jeffreyalaw, 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines
2023-12-15 18:53 [PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation Edwin Lu
@ 2023-12-15 18:53 ` Edwin Lu
2023-12-20 18:50 ` Jeff Law
2023-12-15 18:53 ` [PATCH 2/3][RFC] RISC-V: Add vector related reservations Edwin Lu
2023-12-15 18:53 ` [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation Edwin Lu
2 siblings, 1 reply; 13+ messages in thread
From: Edwin Lu @ 2023-12-15 18:53 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng, jeffreyalaw, 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3][RFC] RISC-V: Add vector related reservations
2023-12-15 18:53 [PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation Edwin Lu
2023-12-15 18:53 ` [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines Edwin Lu
@ 2023-12-15 18:53 ` Edwin Lu
2023-12-20 18:57 ` Jeff Law
2023-12-15 18:53 ` [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation Edwin Lu
2 siblings, 1 reply; 13+ messages in thread
From: Edwin Lu @ 2023-12-15 18:53 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng, jeffreyalaw, Edwin Lu, Robin Dapp
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation
2023-12-15 18:53 [PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation Edwin Lu
2023-12-15 18:53 ` [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines Edwin Lu
2023-12-15 18:53 ` [PATCH 2/3][RFC] RISC-V: Add vector related reservations Edwin Lu
@ 2023-12-15 18:53 ` Edwin Lu
2023-12-20 18:57 ` Jeff Law
2 siblings, 1 reply; 13+ messages in thread
From: Edwin Lu @ 2023-12-15 18:53 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng, jeffreyalaw, 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines
2023-12-15 18:53 ` [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines Edwin Lu
@ 2023-12-20 18:50 ` Jeff Law
2023-12-20 22:11 ` Edwin Lu
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Law @ 2023-12-20 18:50 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, kito.cheng
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
2023-12-15 18:53 ` [PATCH 2/3][RFC] RISC-V: Add vector related reservations Edwin Lu
@ 2023-12-20 18:57 ` Jeff Law
2023-12-20 22:55 ` Edwin Lu
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Law @ 2023-12-20 18:57 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, kito.cheng, Robin Dapp
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation
2023-12-15 18:53 ` [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation Edwin Lu
@ 2023-12-20 18:57 ` Jeff Law
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Law @ 2023-12-20 18:57 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, kito.cheng
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines
2023-12-20 18:50 ` Jeff Law
@ 2023-12-20 22:11 ` Edwin Lu
2023-12-20 22:11 ` Edwin Lu
2023-12-21 6:59 ` Jeff Law
0 siblings, 2 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-20 22:11 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: gnu-toolchain, kito.cheng
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines
2023-12-20 22:11 ` Edwin Lu
@ 2023-12-20 22:11 ` Edwin Lu
2023-12-21 6:59 ` Jeff Law
1 sibling, 0 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-20 22:11 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
2023-12-20 18:57 ` Jeff Law
@ 2023-12-20 22:55 ` Edwin Lu
2023-12-20 22:55 ` Edwin Lu
2023-12-26 21:21 ` Edwin Lu
0 siblings, 2 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-20 22:55 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: gnu-toolchain, kito.cheng, Robin Dapp
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
2023-12-20 22:55 ` Edwin Lu
@ 2023-12-20 22:55 ` Edwin Lu
2023-12-26 21:21 ` Edwin Lu
1 sibling, 0 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-20 22:55 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, kito.cheng, Robin Dapp
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines
2023-12-20 22:11 ` Edwin Lu
2023-12-20 22:11 ` Edwin Lu
@ 2023-12-21 6:59 ` Jeff Law
1 sibling, 0 replies; 13+ messages in thread
From: Jeff Law @ 2023-12-21 6:59 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, kito.cheng
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
2023-12-20 22:55 ` Edwin Lu
2023-12-20 22:55 ` Edwin Lu
@ 2023-12-26 21:21 ` Edwin Lu
1 sibling, 0 replies; 13+ messages in thread
From: Edwin Lu @ 2023-12-26 21:21 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: gnu-toolchain, kito.cheng, Robin Dapp
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-12-26 21:21 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15 18:53 [PATCH 0/3][RFC] RISC-V: Associate typed insns to dfa reservation Edwin Lu
2023-12-15 18:53 ` [PATCH 1/3][RFC] RISC-V: Add non-vector types to pipelines Edwin Lu
2023-12-20 18:50 ` Jeff Law
2023-12-20 22:11 ` Edwin Lu
2023-12-20 22:11 ` Edwin Lu
2023-12-21 6:59 ` Jeff Law
2023-12-15 18:53 ` [PATCH 2/3][RFC] RISC-V: Add vector related reservations Edwin Lu
2023-12-20 18:57 ` Jeff Law
2023-12-20 22:55 ` Edwin Lu
2023-12-20 22:55 ` Edwin Lu
2023-12-26 21:21 ` Edwin Lu
2023-12-15 18:53 ` [PATCH 3/3][RFC] RISC-V: Enable assert for insn_has_dfa_reservation Edwin Lu
2023-12-20 18:57 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).