Sure @Palmer Dabbelt ,sent in a different thread email with updated patch. Thank you ~U
-----Original Message----- From: Palmer Dabbelt <pal...@dabbelt.com> Sent: 08 May 2025 23:38 To: Umesh Kalappa <ukala...@mips.com> Cc: jeffreya...@gmail.com; gcc-patches@gcc.gnu.org; kito.ch...@sifive.com; jesse.hu...@sifive.com; Andrew Waterman <and...@sifive.com> Subject: [EXTERNAL]RE: [PATCH ]RISCV :Added MIPS P8700 Subtarget On Thu, 08 May 2025 08:53:18 PDT (-0700), ukala...@mips.com wrote: > Hi All , > > We have couple of patch series that enables the P8700 tune for RISCV core to > upstream for GCC mainline. > > It will be good to hear from you guys on the patch feedback It's kind of hard to read because your patch is getting mangled by some email-related thing. Can you try using git-send-email to send a clean v2 of the patch? > > Thank you in advance > ~U > > > > -----Original Message----- > From: Umesh Kalappa > Sent: 03 May 2025 11:27 > To: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org; > pal...@dabbelt.com > Cc: kito.ch...@sifive.com; Jesse Huang <jesse.hu...@sifive.com>; > and...@sifive.com > Subject: Re: [PATCH]RISCV :Added MIPS P8700 Subtarget > > 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
