On 08/09/2026 09:31, Richard Sandiford wrote:
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.

I believe those costs would be enough, but the later patch to the RDFFR RTL representation with sub-registers breaks that costing (I believe).

This is admittedly a hack, but I'm not certain how to fix the existing costing.

KR,
Alfie

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.>
Thanks,
Richard

Reply via email to