| Message ID | 20231215185328.794425-2-ewlu@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Associate typed insns to dfa reservation | expand |
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
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
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
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")
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(-)