> -----Original Message-----
> From: Alfie Richards <[email protected]>
> Sent: 08 August 2026 00:11
> To: [email protected]
> Cc: Alex Coplan <[email protected]>; Alice Carlotti
> <[email protected]>; [email protected];
> [email protected]; Tamar Christina <[email protected]>;
> [email protected]; [email protected]; [email protected]; Alfie
> Richards <[email protected]>
> Subject: [PATCH v2 02/13] aarch64: Make reads/writes to FFR expensive
>
> With later changes to RDFFR to use subregisters we see forward prop
> begin to duplicate FFR reads. This change prevents that.
>
Do you have a testcase that shows this?
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.cc (aarch64_insn_cost): Add extra costs
> for FFR reads/writes.
> ---
> gcc/config/aarch64/aarch64.cc | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 3041a6ee62a..e7522e49efd 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -16989,8 +16989,16 @@ aarch64_frame_allocation_cost
> (frame_cost_type,
> static int
> aarch64_insn_cost (rtx_insn *insn, bool speed)
> {
> + int extra_cost = 0;
> +
> + /* Reads and sets to FFR are expensive. Mark them as such to prevent
> + duplication. */
> + if (recog_memoized (insn) != -1
> + && get_attr_sve_type (insn) == SVE_TYPE_SVE_FFR)
> + extra_cost = 100;
> +
That seems quite expensive. Does it really need to be that much?
A testcase would be nice :)
I can certainly believe the cost of RDFFR should not be the default cost,
but I think this is in the wrong place and it shouldn't use the scheduling type
attributes. The above for instance also makes setffr more expensive.
Instead I think this should be in aarch64_rtx_costs in the branch that's
costing a full INSN, (i.e. the case SET: ) but to identify an rdffr as an
instruction with a predicate destination and FFRT_REGNUM mentioned
in the source.
Then just use the normal COSTS_N_INSN macro to scale costs to insn costs
and use the same turning param you introduce later on. So something like
static int
aarch64_ffr_op_cost (rtx set)
{
if (GET_CODE (set) != SET)
return 0;
rtx dest = SET_DEST (set);
rtx x = SET_SRC (set);
if (GET_CODE (x) == UNSPEC
&& XINT (x, 1) == UNSPEC_PTEST
&& refers_to_regno_p (FFRT_REGNUM, x))
return COSTS_N_INSNS (aarch64_tune_params.vec_costs->sve->rdffr_cost);
if (REG_P (dest)
&& aarch64_sve_pred_mode_p (GET_MODE (dest))
&& refers_to_regno_p (FFRT_REGNUM, x))
return COSTS_N_INSNS (aarch64_tune_params.vec_costs->sve->rdffr_cost);
return 0;
}
> if (rtx set = single_set (insn))
> - return set_rtx_cost (set, speed);
> + return set_rtx_cost (set, speed) + extra_cost;
>
> /* If the instruction does multiple sets in parallel, use the cost
> of the most expensive set. This copes with instructions that set
> @@ -17005,10 +17013,10 @@ aarch64_insn_cost (rtx_insn *insn, bool
> speed)
> if (GET_CODE (x) == SET)
> max_cost = std::max (max_cost, set_rtx_cost (x, speed));
> }
> - return max_cost;
> + return max_cost + extra_cost;
> }
>
> - return pattern_cost (pat, speed);
> + return pattern_cost (pat, speed) + extra_cost;
> }
>
> /* Implement TARGET_INIT_BUILTINS. */
> --
> 2.43.0
