Hi @Jeff Law and @pal...@dabbelt.com , Please do needful by reviewing the below changes and helps us to upstream the same .
Thank you ~U -----Original Message----- From: Umesh Kalappa Sent: 29 April 2025 16:16 To: Umesh Kalappa <ukala...@mips.com>; Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org Cc: kito.ch...@sifive.com; Jesse Huang <jesse.hu...@sifive.com>; pal...@dabbelt.com; and...@sifive.com Subject: RE: [EXTERNAL]Re: [PATCH]RISCV :Added MIPS P8700 Subtarget Hi all, Here is the updated patch that address some of the @Jeff Law comments . P8700 don't have a vector engine and we support the insns type till https://github.com/gcc-mirror/gcc/blob/master/gcc/config/riscv/riscv.md#L358 and schedule module enabled the same . --- gcc/config/riscv/mips-p8700.md | 139 +++++++++++++++++++++++++++++++ gcc/config/riscv/riscv-cores.def | 5 ++ gcc/config/riscv/riscv-opts.h | 3 +- gcc/config/riscv/riscv.cc | 22 +++++ gcc/config/riscv/riscv.md | 3 +- 5 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 gcc/config/riscv/mips-p8700.md diff --git a/gcc/config/riscv/mips-p8700.md b/gcc/config/riscv/mips-p8700.md new file mode 100644 index 00000000000..11d0b1ca793 --- /dev/null +++ b/gcc/config/riscv/mips-p8700.md @@ -0,0 +1,139 @@ +;; DFA-based pipeline description for MIPS P8700. +;; +;; Copyright (C) 2025 Free Software Foundation, Inc. +;; +;; This file is part of GCC. +;; +;; GCC is free software; you can redistribute it and/or modify it ;; +under the terms of the GNU General Public License as published ;; by +the Free Software Foundation; either version 3, or (at your ;; option) +any later version. + +;; GCC is distributed in the hope that it will be useful, but WITHOUT +;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY +;; or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public ;; +License for more details. + +;; You should have received a copy of the GNU General Public License ;; +along with GCC; see the file COPYING3. If not see ;; +<http://www.gnu.org/licenses/>. + +(define_automaton "mips_p8700_agen_alq_pipe, mips_p8700_mdu_pipe, +mips_p8700_fpu_pipe") + +;; The address generation queue (AGQ) has AL2, CTISTD and LDSTA pipes +(define_cpu_unit "mips_p8700_agq, mips_p8700_al2, mips_p8700_ctistd, mips_p8700_lsu" + "mips_p8700_agen_alq_pipe") + +(define_cpu_unit "mips_p8700_gpmul, mips_p8700_gpdiv" +"mips_p8700_mdu_pipe") + +;; The arithmetic-logic-unit queue (ALQ) has ALU pipe (define_cpu_unit +"mips_p8700_alq, mips_p8700_alu" "mips_p8700_agen_alq_pipe") + +;; The floating-point-unit queue (FPQ) has short and long pipes +(define_cpu_unit "mips_p8700_fpu_short, mips_p8700_fpu_long" +"mips_p8700_fpu_pipe") + +;; Long FPU pipeline. +(define_cpu_unit "mips_p8700_fpu_apu" "mips_p8700_fpu_pipe") + +(define_reservation "mips_p8700_agq_al2" "mips_p8700_agq, +mips_p8700_al2") (define_reservation "mips_p8700_agq_ctistd" +"mips_p8700_agq, mips_p8700_ctistd") (define_reservation +"mips_p8700_agq_lsu" "mips_p8700_agq, mips_p8700_lsu") +(define_reservation "mips_p8700_alq_alu" "mips_p8700_alq, +mips_p8700_alu") + +;; +;; FPU pipe +;; + +(define_insn_reservation "mips_p8700_fpu_fadd" 4 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fadd")) + "mips_p8700_fpu_long, mips_p8700_fpu_apu") + +(define_insn_reservation "mips_p8700_fpu_fabs" 2 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fcmp,fmove")) + "mips_p8700_fpu_short, mips_p8700_fpu_apu") + +(define_insn_reservation "mips_p8700_fpu_fload" 8 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fpload")) + "mips_p8700_agq_lsu") + +(define_insn_reservation "mips_p8700_fpu_fstore" 1 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fpstore")) + "mips_p8700_agq_lsu") + +(define_insn_reservation "mips_p8700_fpu_fmadd" 8 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fmadd")) + "mips_p8700_fpu_long, mips_p8700_fpu_apu") + +(define_insn_reservation "mips_p8700_fpu_fmul" 5 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fmul")) + "mips_p8700_fpu_long, mips_p8700_fpu_apu") + +(define_insn_reservation "mips_p8700_fpu_div" 17 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fdiv,fsqrt")) + "mips_p8700_fpu_long, mips_p8700_fpu_apu*17") + +(define_insn_reservation "mips_p8700_fpu_fcvt" 4 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "fcvt,fcvt_i2f,fcvt_f2i")) + "mips_p8700_fpu_long, mips_p8700_fpu_apu") + +(define_insn_reservation "mips_p8700_fpu_fmtc" 7 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "mtc")) + "mips_p8700_agq_lsu") + +(define_insn_reservation "mips_p8700_fpu_fmfc" 7 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "mfc")) + "mips_p8700_agq_lsu") + +;; +;; Integer pipe +;; + +(define_insn_reservation "mips_p8700_int_load" 4 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "load")) + "mips_p8700_agq_lsu") + +(define_insn_reservation "mips_p8700_int_store" 3 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "store")) + "mips_p8700_agq_lsu") + +(define_insn_reservation "mips_p8700_int_arith_1" 1 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" +"unknown,const,arith,shift,slt,multi,auipc,logical,move,bitmanip,min,ma +x,minu,maxu,clz,ctz,rotate,atomic,condmove,crypto,mvpair,zicond")) + "mips_p8700_alq_alu | mips_p8700_agq_al2") + +(define_insn_reservation "mips_p8700_int_nop" 0 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "nop")) + "mips_p8700_alq_alu | mips_p8700_agq_al2") + +(define_insn_reservation "mips_p8700_dsp_mult" 4 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "imul,cpop,clmul")) + "mips_p8700_gpmul") + +(define_insn_reservation "mips_p8700_int_div" 8 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "idiv")) + "mips_p8700_gpdiv*5") + +(define_insn_reservation "mips_p8700_int_branch" 1 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "branch,jump,ret,sfb_alu,trap")) + "mips_p8700_agq_ctistd") + +(define_insn_reservation "mips_p8700_int_call" 2 + (and (eq_attr "tune" "mips_p8700") + (eq_attr "type" "call,jalr")) + "mips_p8700_agq_ctistd") diff --git a/gcc/config/riscv/riscv-cores.def b/gcc/config/riscv/riscv-cores.def index e31afc3fe70..118fef23cad 100644 --- a/gcc/config/riscv/riscv-cores.def +++ b/gcc/config/riscv/riscv-cores.def @@ -50,6 +50,7 @@ RISCV_TUNE("xt-c920v2", generic, generic_ooo_tune_info) RISCV_TUNE("xiangshan-nanhu", xiangshan, xiangshan_nanhu_tune_info) RISCV_TUNE("generic-ooo", generic_ooo, generic_ooo_tune_info) RISCV_TUNE("size", generic, optimize_size_tune_info) +RISCV_TUNE("mips-p8700", mips_p8700, mips_p8700_tune_info) #undef RISCV_TUNE @@ -152,4 +153,8 @@ RISCV_CORE("xiangshan-nanhu", "rv64imafdc_zba_zbb_zbc_zbs_" "zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh_" "svinval_zicbom_zicboz", "xiangshan-nanhu") + +RISCV_CORE("mips-p8700", "rv64imafd_zicsr_zmmul_" + "zaamo_zalrsc_zba_zbb", + "mips-p8700") #undef RISCV_CORE diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h index 26fe228e0f8..3ae284b0d95 100644 --- a/gcc/config/riscv/riscv-opts.h +++ b/gcc/config/riscv/riscv-opts.h @@ -58,7 +58,8 @@ enum riscv_microarchitecture_type { sifive_p400, sifive_p600, xiangshan, - generic_ooo + generic_ooo, + mips_p8700, }; extern enum riscv_microarchitecture_type riscv_microarchitecture; diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index bad59e248d0..1e116061d3c 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -642,6 +642,28 @@ static const struct riscv_tune_param optimize_size_tune_info = { NULL, /* loop_align */ }; +/* Costs to use when optimizing for MIPS P8700 */ static const struct +riscv_tune_param mips_p8700_tune_info = { + {COSTS_N_INSNS (4), COSTS_N_INSNS (4)}, /* fp_add */ + {COSTS_N_INSNS (5), COSTS_N_INSNS (5)}, /* fp_mul */ + {COSTS_N_INSNS (17), COSTS_N_INSNS (17)}, /* fp_div */ + {COSTS_N_INSNS (5), COSTS_N_INSNS (5)}, /* int_mul */ + {COSTS_N_INSNS (8), COSTS_N_INSNS (8)}, /* int_div */ + 4, /* issue_rate */ + 8, /* branch_cost */ + 4, /* memory_cost */ + 8, /* fmv_cost */ + true, /* slow_unaligned_access */ + false, /* vector_unaligned_access */ + true, /* use_divmod_expansion */ + false, /* overlap_op_by_pieces */ + RISCV_FUSE_NOTHING, /* fusible_ops */ + NULL, /* vector cost */ + NULL, /* function_align */ + NULL, /* jump_align */ + NULL, /* loop_align */ +}; + static bool riscv_avoid_shrink_wrapping_separate (); static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *); static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *); diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index eec96875f96..ab5da98b437 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -669,7 +669,7 @@ ;; Microarchitectures we know how to tune for. ;; Keep this in sync with enum riscv_microarchitecture. (define_attr "tune" - "generic,sifive_7,sifive_p400,sifive_p600,xiangshan,generic_ooo" + "generic,sifive_7,sifive_p400,sifive_p600,xiangshan,generic_ooo,mips_p8700" (const (symbol_ref "((enum attr_tune) riscv_microarchitecture)"))) ;; Describe a user's asm statement. @@ -4840,3 +4840,4 @@ (include "zc.md") (include "corev.md") (include "xiangshan.md") +(include "mips-p8700.md") -- 2.18.0 Thank you ~U -----Original Message----- From: Umesh Kalappa <ukala...@mips.com> Sent: 22 April 2025 09:54 To: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org Cc: kito.ch...@sifive.com; Jesse Huang <jesse.hu...@sifive.com>; pal...@dabbelt.com; and...@sifive.com Subject: RE: [EXTERNAL]Re: [PATCH]RISCV :Added MIPS P8700 Subtarget Thank you @Jeff Law for the suggestions and >> Just quickly scanning the insn reservations, I suspect you're missing many >> cases and the compiler will trip assertion failures if you are missing cases. Sure will look at it . >> You might want to look at these values more closely. If you have >> questions about how the compiler uses them to make decisions, just >> ask Sure ,and lets us tune the same and reach out here for future questions . ~U -----Original Message----- From: Jeff Law <jeffreya...@gmail.com> Sent: 18 April 2025 22:42 To: Umesh Kalappa <ukala...@mips.com>; gcc-patches@gcc.gnu.org Cc: kito.ch...@sifive.com; Jesse Huang <jesse.hu...@sifive.com>; pal...@dabbelt.com; and...@sifive.com Subject: [EXTERNAL]Re: [PATCH]RISCV :Added MIPS P8700 Subtarget On 4/11/25 6:02 AM, Umesh Kalappa wrote: > This is the first patch from the two-patch series, where configured > gcc for P8700 micro architecture in the first patch and Tested with dejagnu > riscv.exp tests for --mtune=mips-p8700. > > P8700 is a high-performance processor from MIPS by extending RISCV. The > following changes enable P8700 processor for RISCV. > > * config/riscv/riscv-cores.def(RISCV_TUNE):Added mips-p8700 tune. > * config/riscv/riscv-opts.h(riscv_microarchitecture_type):Likewise > * config/riscv/riscv.cc(riscv_tune_param):Added insns costs p8700 tune. > * config/riscv/riscv.md:Added p8700 tune for insn attribute. > * config/riscv/mips-p8700.md:New File for mips-p8700 pipeline > description > --- > gcc/config/riscv/mips-p8700.md | 139 +++++++++++++++++++++++++++++++ > gcc/config/riscv/riscv-cores.def | 1 + > gcc/config/riscv/riscv-opts.h | 3 +- > gcc/config/riscv/riscv.cc | 22 +++++ > gcc/config/riscv/riscv.md | 3 +- > 5 files changed, 166 insertions(+), 2 deletions(-) create mode > 100644 gcc/config/riscv/mips-p8700.md > > diff --git a/gcc/config/riscv/mips-p8700.md > b/gcc/config/riscv/mips-p8700.md new file mode 100644 index > 00000000000..11d0b1ca793 > --- /dev/null > +++ b/gcc/config/riscv/mips-p8700.md > @@ -0,0 +1,139 @@ > +;; DFA-based pipeline description for MIPS P8700. > +;; > +;; Copyright (C) 2025 Free Software Foundation, Inc. > +;; > +;; This file is part of GCC. > +;; > +;; GCC is free software; you can redistribute it and/or modify it ;; > +under the terms of the GNU General Public License as published ;; by > +the Free Software Foundation; either version 3, or (at your ;; > +option) any later version. It looks like your patchfile is getting corrupted. THe ';;' comment markers should have been at the beginning of each line. This kind of problem seems pervasive in your patch and certainly makes it harder to read/evaluate as comments are mixed on the same line as the various parts of pipeline description. > + > +(define_automaton "mips_p8700_agen_alq_pipe, mips_p8700_mdu_pipe, > +mips_p8700_fpu_pipe") We don't typically see things broken up like this. Typically we only use distinct automaton for things like div/sqrt which can make the DFA unreasonably large and cause gen* to run for unreasonably long times. Just quickly scanning the insn reservations, I suspect you're missing many cases and the compiler will trip assertion failures if you are missing cases. Essentially every insn type must map to a reservation, even types that your design doesn't support. I would suggest walking through each type in riscv.md and making sure each and every one maps to an insn reservation. Feel free to make a dummy reservation for things you don't care about. > +/* Costs to use when optimizing for MIPS P8700. */ static const > +struct riscv_tune_param mips_p8700_tune_info = { > + {COSTS_N_INSNS (4), COSTS_N_INSNS (4)}, /* fp_add */ > + {COSTS_N_INSNS (5), COSTS_N_INSNS (5)}, /* fp_mul */ > + {COSTS_N_INSNS (17), COSTS_N_INSNS (17)}, /* fp_div */ > + {COSTS_N_INSNS (5), COSTS_N_INSNS (5)}, /* int_mul */ > + {COSTS_N_INSNS (8), COSTS_N_INSNS (8)}, /* int_div */ > + 4, /* issue_rate */ > + 8, /* branch_cost */ > + 4, /* memory_cost */ > + 8, /* fmv_cost */ > + true, /* slow_unaligned_access */ > + false, /* > vector_unaligned_access */ > + false, /* use_divmod_expansion */ > + false, /* overlap_op_by_pieces */ > + RISCV_FUSE_NOTHING, /* fusible_ops */ > + NULL, /* vector cost */ > + NULL, /* function_align */ > + NULL, /* jump_align */ > + NULL, /* loop_align */ A bit surprised by some of these values. For example, your code specifies no fusion, has a fairly high cost for integer division, but does not ask for divmod expansion. Your DFA shows div/mod as partially pipelined. So I would expect divmod expansion to be profitable. You might want to look at these values more closely. If you have questions about how the compiler uses them to make decisions, just ask. Jeff
