Thanks Robin.
> The series looks generally reasonable to me now, thanks. But if we have a
> parameter that sets GR2VR IMHO it should affect all of its uses. That means
> we
> should either adjust the regmove costs directly or use the new helper
> whenever
> GR2VR is currently used.
> As we only have a single regmove struct it shouldn't be too difficult to
> adjust
> it directly.
I added a helper here because of
1. those static const var initialized before options, can hardly initialize
correct.
2. The --param is somehow experimental, thus I prefer to keep the const GR2VR
in static structure as is.
I will append a new patch,aka let the reference goes to the new helper if that
is OK.
> Regarding naming - I don't have a strong opinion here but GPR is a more
> common
> term than GR. So how about just --param=gpr2vr-cost=?
I see, shall we rename all GR to GPR? I mean we have GR2VR define already.
Pan
-----Original Message-----
From: Robin Dapp <rdapp....@gmail.com>
Sent: Monday, May 5, 2025 3:54 PM
To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; jeffreya...@gmail.com;
rdapp....@gmail.com; Chen, Ken <ken.c...@intel.com>; Liu, Hongtao
<hongtao....@intel.com>; Robin Dapp <rdapp....@gmail.com>
Subject: Re: [PATCH v1 1/5] RISC-V: Add new option --param=rvv-gr2vr-cost= for
rvv insn
Hi Pan,
> During investigate the combine from vec_dup and vop.vv into
> vop.vx, we need to depend on the cost of the insn operate
> from the gr to vr, for example, vadd.vx. Thus, for better
> control and test, we introduce a new option, aka below:
>
> --param=rvv-gr2vr-cost=<unsigned int>
> +static inline int
> +get_vector_gr2vr_cost ()
> +{
> + int cost = get_vector_costs ()->regmove->GR2VR;
> +
> + if (rvv_gr2vr_cost != RVV_GR2VR_COST_UNPROVIDED)
> + cost = rvv_gr2vr_cost;
> +
> + return cost;
> +}
The series looks generally reasonable to me now, thanks. But if we have a
parameter that sets GR2VR IMHO it should affect all of its uses. That means we
should either adjust the regmove costs directly or use the new helper whenever
GR2VR is currently used.
As we only have a single regmove struct it shouldn't be too difficult to adjust
it directly.
Regarding naming - I don't have a strong opinion here but GPR is a more common
term than GR. So how about just --param=gpr2vr-cost=?
--
Regards
Robin
