> -----Original Message----- > From: Richard Sandiford <[email protected]> > Sent: 08 September 2026 09:31 > To: Tamar Christina <[email protected]> > Cc: Alfie Richards <[email protected]>; [email protected]; Alex > Coplan <[email protected]>; Alice Carlotti <[email protected]>; > [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH v2 02/13] aarch64: Make reads/writes to FFR expensive > > Tamar Christina <[email protected]> writes: > >> -----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. > > Agreed that this should be unidirectional FWIW. It's unfortunate that > the (already high) move costs aren't enough for move instructions, > and that we need this other mechanism as well. > > > 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; > > } > > FWIW, I think insn_costs is preferred over rtx_costs for this kind of thing. > The hook was specifically added to work around problems with rtx_costs, > and also to enable attributes to be used in preference to manually > reproducing parts of genrecog.
Sure, happy to do insn_costs if you think it's better, I asked to move it since doing it in insn_costs you do have to be careful about costing it inside a parallel where you likely want the existing behavior of rtx_costs which takes the maximum. So not hard to do, but looked like it was re-doing the work of rtx_costs. Thanks, Tamar > > Thanks, > Richard
