Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
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
Re: [PATCH 2/3][RFC] RISC-V: Add vector related reservations
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 Co-authored-by: Robin Dapp --- 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 "\ " 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
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 Co-authored-by: Robin Dapp --- 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
[PATCH 2/3][RFC] RISC-V: Add vector related reservations
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 Co-authored-by: Robin Dapp --- 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
