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

[Jeff Law <jeffreyalaw@gmail.com>]

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