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
