aarch64: Fix invalid CSE for RDRFFS [PR 126629]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
Commit Message
Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
the FFR loads to the associated FFR updates.
This isn't quite perfectly true, as the hardware presumably could set
return different FFR values from the exact same load happening twice, but that
is a nonsense situation where the optimisation is valid.
PR 126629
gcc/ChangeLog:
* config/aarch64/aarch64-sve-builtins-base.cc:
(svldff1_gather_impl::expand): Remove gen_aarch64_update_ffr_for_load.
(svldff1_svldff1_gather_extend::expand): Ditto.
(svldxf1_impl::expand): Ditto.
(svldxf1_extend_impl::expand): Ditto.
* config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load): Remove.
(*aarch64_update_ffr_for_continuous_load): New.
(*aarch64_update_ffr_for_gather_load): New.
(@aarch64_ld<fn>f1<mode>): New.
(*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
(@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_I:mode>): New.
(*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_I:mode>): Change to not be expanded directly.
(@aarch64_ldff1_gather<mode>): New.
(*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
(@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_NARROW:mode>): New.
(*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_NARROW:mode>): Change to not be expanded directly.
(@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_NARROW:mode>): New.
(*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_NARROW:mode>): Change to not be expanded directly.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/sve/pr126629.c: New test.
* gcc.target/aarch64/sve/pr126629_extend.c: New test.
* gcc.target/aarch64/sve/pr126629_gather.c: New test.
* gcc.target/aarch64/sve/pr126629_gather2.c: New test.
-- >8 --
Bootstrapped and regression tested on aarch64.
Okay for master and backport?
Thanks,
Alfie
---
.../aarch64/aarch64-sve-builtins-base.cc | 4 -
gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
.../gcc.target/aarch64/sve/pr126629.c | 23 ++
.../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
.../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
.../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
6 files changed, 279 insertions(+), 25 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
Comments
> -----Original Message-----
> From: Alfie Richards <alfie.richards@arm.com>
> Sent: 05 August 2026 16:42
> To: gcc-patches@gcc.gnu.org
> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Tamar Christina <Tamar.Christina@arm.com>;
> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> the FFR loads to the associated FFR updates.
>
> This isn't quite perfectly true, as the hardware presumably could set
> return different FFR values from the exact same load happening twice, but that
> is a nonsense situation where the optimisation is valid.
>
Hi Alfie,
I think the idea is good, but I don't quite like that now every first faulting load shape
needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
If I'm not mistaken the problem is that CSE lib essentially sees
(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)]
UNSPEC_UPDATE_FFR))
(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)]
UNSPEC_UPDATE_FFR))
And because these are pseudo registers and we only have one of them the REGNOs are the same
and so it assumes they are the same. The patch fixes it by adding the load arguments to the
UNSPEC_UPDATE_FFR to make them unique to the load.
But I think we can fix this simpler by just adding a unique token to the aarch64_update_ffr_for_load.
Like
(define_insn "aarch64_update_ffr_for_load"
[(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)
(match_operand 0 "const_int_operand" "n")]
UNSPEC_UPDATE_FFR))]
Then change
emit_insn (gen_aarch64_update_ffr_for_load ());
into
emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
the exact value of which doesn't really matter as long as it's unique (maybe make a helper).
That should give us a unique aarch64_update_ffr_for_load and also like your patch still
allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
Thanks,
Tamar
> PR 126629
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-sve-builtins-base.cc:
> (svldff1_gather_impl::expand): Remove
> gen_aarch64_update_ffr_for_load.
> (svldff1_svldff1_gather_extend::expand): Ditto.
> (svldxf1_impl::expand): Ditto.
> (svldxf1_extend_impl::expand): Ditto.
> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> Remove.
> (*aarch64_update_ffr_for_continuous_load): New.
> (*aarch64_update_ffr_for_gather_load): New.
> (@aarch64_ld<fn>f1<mode>): New.
> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> PARTIAL_I:mode>): New.
> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> ARTIAL_I:mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather<mode>): New.
> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> <VNx4_NARROW:mode>): New.
> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> VNx4_NARROW:mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> <VNx2_NARROW:mode>): New.
> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> VNx2_NARROW:mode>): Change to not be expanded directly.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/sve/pr126629.c: New test.
> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
>
> -- >8 --
>
> Bootstrapped and regression tested on aarch64.
>
> Okay for master and backport?
>
> Thanks,
> Alfie
>
> ---
> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> 6 files changed, 279 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index 7f047bb6468..186f223ac63 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -2001,7 +2001,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> e.prepare_gather_address_operands (1);
> /* Put the predicate last, since ldff1_gather uses the same operand
> @@ -2023,7 +2022,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> e.prepare_gather_address_operands (1);
> /* Put the predicate last, since ldff1_gather uses the same operand
> @@ -2075,7 +2073,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> machine_mode mode = e.vector_mode (0);
> return e.use_contiguous_load_insn (code_for_aarch64_ldf1 (m_unspec,
> mode));
> @@ -2103,7 +2100,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code (),
> e.vector_mode (0),
> diff --git a/gcc/config/aarch64/aarch64-sve.md
> b/gcc/config/aarch64/aarch64-sve.md
> index 105b34eb8fa..1e131cc404a 100644
> --- a/gcc/config/aarch64/aarch64-sve.md
> +++ b/gcc/config/aarch64/aarch64-sve.md
> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> ;; so that the FFR value is live on entry to the region and so that the FFR
> ;; value visibly changes within the region. This is used (possibly multiple
> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> -(define_insn "aarch64_update_ffr_for_load"
> +(define_insn "*aarch64_update_ffr_for_continuous_load"
> [(set (reg:VNx16BI FFR_REGNUM)
> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> - (reg:VNx16BI FFR_REGNUM)]
> UNSPEC_UPDATE_FFR))]
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_operand 0 "" "X")
> + (match_operand 1 "" "X")
> + ] UNSPEC_UPDATE_FFR))]
> + "TARGET_SVE"
> + ""
> + [(set_attr "type" "no_insn")]
> +)
> +
> +(define_insn "*aarch64_update_ffr_for_gather_load"
> + [(set (reg:VNx16BI FFR_REGNUM)
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_operand 0 "" "X")
> + (match_operand 1 "" "X")
> + (match_operand 2 "" "X")
> + (match_operand 3 "" "X")
> + (match_operand 4 "" "X")
> + ] UNSPEC_UPDATE_FFR)
> + )]
> "TARGET_SVE"
> ""
> [(set_attr "type" "no_insn")]
> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> ;; -------------------------------------------------------------------------
>
> ;; Contiguous non-extending first-faulting or non-faulting loads.
> -(define_insn "@aarch64_ld<fn>f1<mode>"
> +(define_expand "@aarch64_ld<fn>f1<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 2)
> + (match_dup 1)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL 0 "register_operand")
> + (unspec:SVE_FULL
> + [(match_operand:<VPRED> 2 "register_operand")
> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> + (reg:VNx16BI FFRT_REGNUM)]
> + SVE_LDFF1_LDNF1))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ld<fn>f1<mode>"
> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> (unspec:SVE_FULL
> [(match_operand:<VPRED> 2 "register_operand" "Upl")
> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> ;; -------------------------------------------------------------------------
>
> ;; Predicated first-faulting or non-faulting load and extend.
> -(define_insn_and_rewrite
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> _I:mode>"
> +(define_expand
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> _I:mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 2)
> + (match_dup 1)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_HSDI 0 "register_operand")
> + (unspec:SVE_HSDI
> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> + (ANY_EXTEND:SVE_HSDI
> + (unspec:SVE_PARTIAL_I
> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> + (match_operand:SVE_PARTIAL_I 1
> "aarch64_sve_ld<fn>f1_operand")
> + (reg:VNx16BI FFRT_REGNUM)]
> + SVE_LDFF1_LDNF1))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE
> + && TARGET_NON_STREAMING
> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> + {
> + if (!CONSTANT_P (operands[3]))
> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> + }
> +)
> +
> +;; Predicated first-faulting or non-faulting load and extend.
> +(define_insn
> "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> I:mode>"
> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> (unspec:SVE_HSDI
> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> "UplDnm")
> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> && TARGET_NON_STREAMING
> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
>
> "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> %2/z, %1"
> - "&& !CONSTANT_P (operands[3])"
> - {
> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> - }
> [(set_attr "sve_type" "sve_load_1reg")]
> )
>
> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
>
> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> ;; 3 is true for unsigned extension and false for signed extension.
> -(define_insn "@aarch64_ldff1_gather<mode>"
> +(define_expand "@aarch64_ldff1_gather<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual load
> + (unspec:SVE_FULL_S
> + [(match_operand:VNx4BI 5 "register_operand")
> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> + (match_operand:VNx4SI 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ldff1_gather<mode>"
> [(set (match_operand:SVE_FULL_S 0 "register_operand")
> (unspec:SVE_FULL_S
> [(match_operand:VNx4BI 5 "register_operand")
> @@ -1938,7 +2029,33 @@ (define_insn "@aarch64_ldff1_gather<mode>"
>
> ;; Predicated first-faulting gather loads for 64-bit elements. The value
> ;; of operand 3 doesn't matter in this case.
> -(define_insn "@aarch64_ldff1_gather<mode>"
> +(define_expand "@aarch64_ldff1_gather<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL_D 0 "register_operand")
> + (unspec:SVE_FULL_D
> + [(match_operand:VNx2BI 5 "register_operand")
> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> + (match_operand:VNx2DI 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ldff1_gather<mode>"
> [(set (match_operand:SVE_FULL_D 0 "register_operand")
> (unspec:SVE_FULL_D
> [(match_operand:VNx2BI 5 "register_operand")
> @@ -2032,7 +2149,40 @@ (define_insn
> "*aarch64_ldff1_gather<mode>_uxtw"
>
> ;; Predicated extending first-faulting gather loads for 32-bit elements.
> ;; Operand 3 is true for unsigned extension and false for signed extension.
> -(define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> NARROW:mode>"
> +(define_expand
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> NARROW:mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:VNx4_WIDE 0 "register_operand")
> + (unspec:VNx4_WIDE
> + [(match_operand:VNx4BI 6 "general_operand")
> + (ANY_EXTEND:VNx4_WIDE
> + (unspec:VNx4_NARROW
> + [(match_operand:VNx4BI 5 "register_operand")
> + (match_operand:DI 1
> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> + (match_operand:VNx4_WIDE 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4
> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {
> + if (!CONSTANT_P (operands[6]))
> + operands[6] = CONSTM1_RTX (VNx4BImode);
> + }
> +)
> +
> +(define_insn
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> ARROW:mode>"
> [(set (match_operand:VNx4_WIDE 0 "register_operand")
> (unspec:VNx4_WIDE
> [(match_operand:VNx4BI 6 "general_operand")
> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> [&w, rk, w, Ui1, i, Upl, UplDnm]
> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s, uxtw
> %p4]
> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> }
> - "&& !CONSTANT_P (operands[6])"
> - {
> - operands[6] = CONSTM1_RTX (VNx4BImode);
> - }
> [(set_attr "sve_type" "sve_gatherload_32")]
> )
>
> ;; Predicated extending first-faulting gather loads for 64-bit elements.
> ;; The value of operand 3 doesn't matter in this case.
> -(define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> NARROW:mode>"
> +(define_expand
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> NARROW:mode>"
> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> + (unspec:VNx2_WIDE
> + [(match_operand:VNx2BI 6 "general_operand")
> + (ANY_EXTEND:VNx2_WIDE
> + (unspec:VNx2_NARROW
> + [(match_operand:VNx2BI 5 "register_operand")
> + (match_operand:DI 1
> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> + (match_operand:VNx2_WIDE 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4
> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {
> + if (!CONSTANT_P (operands[6]))
> + operands[6] = CONSTM1_RTX (VNx2BImode);
> + }
> +)
> +
> +(define_insn
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> ARROW:mode>"
> [(set (match_operand:VNx2_WIDE 0 "register_operand")
> (unspec:VNx2_WIDE
> [(match_operand:VNx2BI 6 "general_operand")
> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> [&w, rk, w, i, i, Upl, UplDnm]
> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d, lsl
> %p4]
> [?w, rk, w, i, i, Upl, UplDnm] ^
> }
> - "&& !CONSTANT_P (operands[6])"
> - {
> - operands[6] = CONSTM1_RTX (VNx2BImode);
> - }
> [(set_attr "sve_type" "sve_gatherload_64")]
> )
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> new file mode 100644
> index 00000000000..64cb3ece330
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint8_t a = svldff1_u8 (pt, p);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint8_t b = svldff1_u8 (pt, q);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u8 (pt, o1, a);
> + svst1_u8 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> new file mode 100644
> index 00000000000..909a9f1e651
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1ub_u32 (pt, p);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1ub_u32 (pt, q);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> new file mode 100644
> index 00000000000..2348082fde5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned *o1, unsigned *o2,
> + svuint32_t offset1, svuint32_t offset2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> new file mode 100644
> index 00000000000..bb9257fe382
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> + unsigned long *r2, unsigned *o1, unsigned *o2,
> + long int offset1, long int offset2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> --
> 2.34.1
Hi Tamar,
Sorry replying from phone so poor formatting.
> I think the idea is good, but I don't quite like that now every first faulting load shape
> needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
I do agree this is the part of the patch I was most uncomfortable with.
> And because these are pseudo registers and we only have one of them the REGNOs are the same
> and so it assumes they are the same. The patch fixes it by adding the load arguments to the
> UNSPEC_UPDATE_FFR to make them unique to the load.
Essentially yes but it’s also because the whole FFR expression tree matches going back to the setffr instructions (I believe)
> That should give us a unique aarch64_update_ffr_for_load and also like your patch still
> allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
Yeah I like that idea, and I think it should work. I did think about something similar but my only hesitation was around the situation where you do two identical loads. I like in that situation with my patch that the duplicate load and rdffr gets removed whereas with the solution you propose we would remove the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big issue as it’s a pretty nonsense case but that’s why I went the way I did.
Thoughts? Happy to take your solution if you’d still rather.
KR,
Alfie
________________________________
From: Tamar Christina <Tamar.Christina@arm.com>
Sent: 07 August 2026 09:23
To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>; ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org <iains@gcc.gnu.org>
Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> -----Original Message-----
> From: Alfie Richards <alfie.richards@arm.com>
> Sent: 05 August 2026 16:42
> To: gcc-patches@gcc.gnu.org
> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Tamar Christina <Tamar.Christina@arm.com>;
> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> the FFR loads to the associated FFR updates.
>
> This isn't quite perfectly true, as the hardware presumably could set
> return different FFR values from the exact same load happening twice, but that
> is a nonsense situation where the optimisation is valid.
>
Hi Alfie,
I think the idea is good, but I don't quite like that now every first faulting load shape
needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
If I’m not mistaken the problem is that CSE lib essentially sees
(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)]
UNSPEC_UPDATE_FFR))
(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)]
UNSPEC_UPDATE_FFR))
And because these are pseudo registers and we only have one of them the REGNOs are the same
and so it assumes they are the same. The patch fixes it by adding the load arguments to the
UNSPEC_UPDATE_FFR to make them unique to the load.
But I think we can fix this simpler by just adding a unique token to the aarch64_update_ffr_for_load.
Like
(define_insn "aarch64_update_ffr_for_load"
[(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)
(match_operand 0 "const_int_operand" "n")]
UNSPEC_UPDATE_FFR))]
Then change
emit_insn (gen_aarch64_update_ffr_for_load ());
into
emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
the exact value of which doesn't really matter as long as it's unique (maybe make a helper).
That should give us a unique aarch64_update_ffr_for_load and also like your patch still
allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
Thanks,
Tamar
> PR 126629
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-sve-builtins-base.cc:
> (svldff1_gather_impl::expand): Remove
> gen_aarch64_update_ffr_for_load.
> (svldff1_svldff1_gather_extend::expand): Ditto.
> (svldxf1_impl::expand): Ditto.
> (svldxf1_extend_impl::expand): Ditto.
> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> Remove.
> (*aarch64_update_ffr_for_continuous_load): New.
> (*aarch64_update_ffr_for_gather_load): New.
> (@aarch64_ld<fn>f1<mode>): New.
> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> PARTIAL_I:mode>): New.
> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> ARTIAL_I:mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather<mode>): New.
> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> <VNx4_NARROW:mode>): New.
> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> VNx4_NARROW:mode>): Change to not be expanded directly.
> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> <VNx2_NARROW:mode>): New.
> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> VNx2_NARROW:mode>): Change to not be expanded directly.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/sve/pr126629.c: New test.
> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
>
> -- >8 --
>
> Bootstrapped and regression tested on aarch64.
>
> Okay for master and backport?
>
> Thanks,
> Alfie
>
> ---
> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> 6 files changed, 279 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index 7f047bb6468..186f223ac63 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -2001,7 +2001,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> e.prepare_gather_address_operands (1);
> /* Put the predicate last, since ldff1_gather uses the same operand
> @@ -2023,7 +2022,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> e.prepare_gather_address_operands (1);
> /* Put the predicate last, since ldff1_gather uses the same operand
> @@ -2075,7 +2073,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> machine_mode mode = e.vector_mode (0);
> return e.use_contiguous_load_insn (code_for_aarch64_ldf1 (m_unspec,
> mode));
> @@ -2103,7 +2100,6 @@ public:
> {
> /* See the block comment in aarch64-sve.md for details about the
> FFR handling. */
> - emit_insn (gen_aarch64_update_ffr_for_load ());
>
> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code (),
> e.vector_mode (0),
> diff --git a/gcc/config/aarch64/aarch64-sve.md
> b/gcc/config/aarch64/aarch64-sve.md
> index 105b34eb8fa..1e131cc404a 100644
> --- a/gcc/config/aarch64/aarch64-sve.md
> +++ b/gcc/config/aarch64/aarch64-sve.md
> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> ;; so that the FFR value is live on entry to the region and so that the FFR
> ;; value visibly changes within the region. This is used (possibly multiple
> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> -(define_insn "aarch64_update_ffr_for_load"
> +(define_insn "*aarch64_update_ffr_for_continuous_load"
> [(set (reg:VNx16BI FFR_REGNUM)
> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> - (reg:VNx16BI FFR_REGNUM)]
> UNSPEC_UPDATE_FFR))]
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_operand 0 "" "X")
> + (match_operand 1 "" "X")
> + ] UNSPEC_UPDATE_FFR))]
> + "TARGET_SVE"
> + ""
> + [(set_attr "type" "no_insn")]
> +)
> +
> +(define_insn "*aarch64_update_ffr_for_gather_load"
> + [(set (reg:VNx16BI FFR_REGNUM)
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_operand 0 "" "X")
> + (match_operand 1 "" "X")
> + (match_operand 2 "" "X")
> + (match_operand 3 "" "X")
> + (match_operand 4 "" "X")
> + ] UNSPEC_UPDATE_FFR)
> + )]
> "TARGET_SVE"
> ""
> [(set_attr "type" "no_insn")]
> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> ;; -------------------------------------------------------------------------
>
> ;; Contiguous non-extending first-faulting or non-faulting loads.
> -(define_insn "@aarch64_ld<fn>f1<mode>"
> +(define_expand "@aarch64_ld<fn>f1<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 2)
> + (match_dup 1)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL 0 "register_operand")
> + (unspec:SVE_FULL
> + [(match_operand:<VPRED> 2 "register_operand")
> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> + (reg:VNx16BI FFRT_REGNUM)]
> + SVE_LDFF1_LDNF1))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ld<fn>f1<mode>"
> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> (unspec:SVE_FULL
> [(match_operand:<VPRED> 2 "register_operand" "Upl")
> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> ;; -------------------------------------------------------------------------
>
> ;; Predicated first-faulting or non-faulting load and extend.
> -(define_insn_and_rewrite
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> _I:mode>"
> +(define_expand
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> _I:mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 2)
> + (match_dup 1)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_HSDI 0 "register_operand")
> + (unspec:SVE_HSDI
> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> + (ANY_EXTEND:SVE_HSDI
> + (unspec:SVE_PARTIAL_I
> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> + (match_operand:SVE_PARTIAL_I 1
> "aarch64_sve_ld<fn>f1_operand")
> + (reg:VNx16BI FFRT_REGNUM)]
> + SVE_LDFF1_LDNF1))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE
> + && TARGET_NON_STREAMING
> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> + {
> + if (!CONSTANT_P (operands[3]))
> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> + }
> +)
> +
> +;; Predicated first-faulting or non-faulting load and extend.
> +(define_insn
> "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> I:mode>"
> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> (unspec:SVE_HSDI
> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> "UplDnm")
> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> && TARGET_NON_STREAMING
> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
>
> "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> %2/z, %1"
> - "&& !CONSTANT_P (operands[3])"
> - {
> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> - }
> [(set_attr "sve_type" "sve_load_1reg")]
> )
>
> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
>
> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> ;; 3 is true for unsigned extension and false for signed extension.
> -(define_insn "@aarch64_ldff1_gather<mode>"
> +(define_expand "@aarch64_ldff1_gather<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual load
> + (unspec:SVE_FULL_S
> + [(match_operand:VNx4BI 5 "register_operand")
> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> + (match_operand:VNx4SI 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ldff1_gather<mode>"
> [(set (match_operand:SVE_FULL_S 0 "register_operand")
> (unspec:SVE_FULL_S
> [(match_operand:VNx4BI 5 "register_operand")
> @@ -1938,7 +2029,33 @@ (define_insn "@aarch64_ldff1_gather<mode>"
>
> ;; Predicated first-faulting gather loads for 64-bit elements. The value
> ;; of operand 3 doesn't matter in this case.
> -(define_insn "@aarch64_ldff1_gather<mode>"
> +(define_expand "@aarch64_ldff1_gather<mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:SVE_FULL_D 0 "register_operand")
> + (unspec:SVE_FULL_D
> + [(match_operand:VNx2BI 5 "register_operand")
> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> + (match_operand:VNx2DI 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {}
> +)
> +
> +(define_insn "*aarch64_ldff1_gather<mode>"
> [(set (match_operand:SVE_FULL_D 0 "register_operand")
> (unspec:SVE_FULL_D
> [(match_operand:VNx2BI 5 "register_operand")
> @@ -2032,7 +2149,40 @@ (define_insn
> "*aarch64_ldff1_gather<mode>_uxtw"
>
> ;; Predicated extending first-faulting gather loads for 32-bit elements.
> ;; Operand 3 is true for unsigned extension and false for signed extension.
> -(define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> NARROW:mode>"
> +(define_expand
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> NARROW:mode>"
> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> + (unspec:VNx16BI
> + [(reg:VNx16BI FFRT_REGNUM)
> + (reg:VNx16BI FFR_REGNUM)
> + (match_dup 5)
> + (match_dup 1)
> + (match_dup 2)
> + (match_dup 3)
> + (match_dup 4)
> + ] UNSPEC_UPDATE_FFR)
> + )
> + (set (match_operand:VNx4_WIDE 0 "register_operand")
> + (unspec:VNx4_WIDE
> + [(match_operand:VNx4BI 6 "general_operand")
> + (ANY_EXTEND:VNx4_WIDE
> + (unspec:VNx4_NARROW
> + [(match_operand:VNx4BI 5 "register_operand")
> + (match_operand:DI 1
> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> + (match_operand:VNx4_WIDE 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4
> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {
> + if (!CONSTANT_P (operands[6]))
> + operands[6] = CONSTM1_RTX (VNx4BImode);
> + }
> +)
> +
> +(define_insn
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> ARROW:mode>"
> [(set (match_operand:VNx4_WIDE 0 "register_operand")
> (unspec:VNx4_WIDE
> [(match_operand:VNx4BI 6 "general_operand")
> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> [&w, rk, w, Ui1, i, Upl, UplDnm]
> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s, uxtw
> %p4]
> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> }
> - "&& !CONSTANT_P (operands[6])"
> - {
> - operands[6] = CONSTM1_RTX (VNx4BImode);
> - }
> [(set_attr "sve_type" "sve_gatherload_32")]
> )
>
> ;; Predicated extending first-faulting gather loads for 64-bit elements.
> ;; The value of operand 3 doesn't matter in this case.
> -(define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> NARROW:mode>"
> +(define_expand
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> NARROW:mode>"
> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> + (unspec:VNx2_WIDE
> + [(match_operand:VNx2BI 6 "general_operand")
> + (ANY_EXTEND:VNx2_WIDE
> + (unspec:VNx2_NARROW
> + [(match_operand:VNx2BI 5 "register_operand")
> + (match_operand:DI 1
> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> + (match_operand:VNx2_WIDE 2 "register_operand")
> + (match_operand:DI 3 "const_int_operand")
> + (match_operand:DI 4
> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> + (mem:BLK (scratch))
> + (reg:VNx16BI FFRT_REGNUM)]
> + UNSPEC_LDFF1_GATHER))]
> + UNSPEC_PRED_X))]
> + "TARGET_SVE && TARGET_NON_STREAMING"
> + {
> + if (!CONSTANT_P (operands[6]))
> + operands[6] = CONSTM1_RTX (VNx2BImode);
> + }
> +)
> +
> +(define_insn
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> ARROW:mode>"
> [(set (match_operand:VNx2_WIDE 0 "register_operand")
> (unspec:VNx2_WIDE
> [(match_operand:VNx2BI 6 "general_operand")
> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> [&w, rk, w, i, i, Upl, UplDnm]
> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d, lsl
> %p4]
> [?w, rk, w, i, i, Upl, UplDnm] ^
> }
> - "&& !CONSTANT_P (operands[6])"
> - {
> - operands[6] = CONSTM1_RTX (VNx2BImode);
> - }
> [(set_attr "sve_type" "sve_gatherload_64")]
> )
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> new file mode 100644
> index 00000000000<tel:00000000000>..64cb3ece330
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint8_t a = svldff1_u8 (pt, p);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint8_t b = svldff1_u8 (pt, q);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u8 (pt, o1, a);
> + svst1_u8 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> new file mode 100644
> index 00000000000<tel:00000000000>..909a9f1e651
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1ub_u32 (pt, p);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1ub_u32 (pt, q);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> new file mode 100644
> index 00000000000<tel:00000000000>..2348082fde5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> + unsigned long *r2, unsigned *o1, unsigned *o2,
> + svuint32_t offset1, svuint32_t offset2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> new file mode 100644
> index 00000000000<tel:00000000000>..bb9257fe382
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include <arm_sve.h>
> +
> + void
> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> + unsigned long *r2, unsigned *o1, unsigned *o2,
> + long int offset1, long int offset2)
> +{
> + svbool_t pt = svptrue_b8 ();
> + svsetffr ();
> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> + svsetffr ();
> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> + svst1_u32 (pt, o1, a);
> + svst1_u32 (pt, o2, b);
> + *r1 = n1;
> + *r2 = n2;
> +}
> +
> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> --
> 2.34.1
Hi Alfie,
The 08/07/2026 11:31, Alfie Richards wrote:
> Hi Tamar,
> Sorry replying from phone so poor formatting.
>
> > I think the idea is good, but I don't quite like that now every first faulting load shape
> > needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
>
> I do agree this is the part of the patch I was most uncomfortable with.
>
> > And because these are pseudo registers and we only have one of them the REGNOs are the same
> > and so it assumes they are the same. The patch fixes it by adding the load arguments to the
> > UNSPEC_UPDATE_FFR to make them unique to the load.
>
> Essentially yes but it’s also because the whole FFR expression tree matches going back to the setffr instructions (I believe)
>
> > That should give us a unique aarch64_update_ffr_for_load and also like your patch still
> > allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
>
> Yeah I like that idea, and I think it should work. I did think about something similar but my only hesitation was around the situation where you do two identical loads. I like in that situation with my patch that the duplicate load and rdffr gets removed whereas with the solution you propose we would remove the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big issue as it’s a pretty nonsense case but that’s why I went the way I did.
>
I'm not sure I understand which scenario this is.
Applying your patch:
#include <arm_sve.h>
void
same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
unsigned char *o2, unsigned long *r1,
unsigned long *r2)
{
svbool_t pt = svptrue_b8 ();
svsetffr ();
svuint8_t a = svldff1_u8 (pt, p);
unsigned long n1 = svcntp_b8 (pt, svrdffr ());
svuint8_t b = svldff1_u8 (pt, p);
unsigned long n2 = svcntp_b8 (pt, svrdffr ());
svst1_u8 (pt, o1, a);
svst1_u8 (pt, o2, b);
*r1 = n1;
*r2 = n2;
}
gives me
same_load_read_same_load_read:
setffr
ptrue p7.b, all
ldff1b z31.b, p7/z, [x0]
rdffr p15.b
ldff1b z30.b, p7/z, [x0]
st1b z31.b, p7, [x1]
st1b z30.b, p7, [x2]
cntp x0, p7, p15.b
rdffr p15.b
str x0, [x3]
cntp x0, p7, p15.b
str x0, [x4]
ret
which is expected since the UNSPEC_UPDATE_FFR can't be eliminated because even your version has a dependendcy on both FFR_NUM and FFRT_NUM.
so the intevening write to FFRT means that the two
RTL sequences are:
First first-fault load:
(insn 12 ...
(set (reg:VNx16BI 85 ffr)
(unspec:VNx16BI [
(reg:VNx16BI 86 ffrt)
(reg:VNx16BI 85 ffr)
(reg:VNx16BI 112)
(mem:VNx16QI (reg/v/f:DI 107 [ p ]) [0 S[16, 16] A8])
] UNSPEC_UPDATE_FFR)) ... {*aarch64_update_ffr_for_continuous_load})
(insn 13 ...
(set (reg/v:VNx16QI 103 [ a ])
(unspec:VNx16QI [
(reg:VNx16BI 112)
(mem:VNx16QI (reg/v/f:DI 107 [ p ]) [0 S[16, 16] A8])
(reg:VNx16BI 86 ffrt)
] UNSPEC_LDFF1)) ... {*aarch64_ldff1vnx16qi})
First svrdffr:
(insn 14 ...
(set (reg:VNx16BI 86 ffrt)
(reg:VNx16BI 85 ffr)) ... {aarch64_copy_ffr_to_ffrt})
(insn 15 ...
(set (reg:VNx16BI 101 [ _1 ])
(reg:VNx16BI 86 ffrt)) ... {aarch64_rdffr})
(insn 16 ...
(set (reg:VNx16BI 86 ffrt)
(unspec:VNx16BI [
(reg:VNx16BI 86 ffrt)
] UNSPEC_UPDATE_FFRT)) ... {aarch64_update_ffrt})
Second first-fault load:
(insn 20 ...
(set (reg:VNx16BI 85 ffr)
(unspec:VNx16BI [
(reg:VNx16BI 86 ffrt)
(reg:VNx16BI 85 ffr)
(reg:VNx16BI 114)
(mem:VNx16QI (reg/v/f:DI 107 [ p ]) [0 S[16, 16] A8])
] UNSPEC_UPDATE_FFR)) ... {*aarch64_update_ffr_for_continuous_load})
(insn 21 ...
(set (reg/v:VNx16QI 105 [ b ])
(unspec:VNx16QI [
(reg:VNx16BI 114)
(mem:VNx16QI (reg/v/f:DI 107 [ p ]) [0 S[16, 16] A8])
(reg:VNx16BI 86 ffrt)
] UNSPEC_LDFF1)) ... {*aarch64_ldff1vnx16qi})
Second svrdffr:
(insn 22 ...
(set (reg:VNx16BI 86 ffrt)
(reg:VNx16BI 85 ffr)) ... {aarch64_copy_ffr_to_ffrt})
(insn 23 ...
(set (reg:VNx16BI 102 [ _2 ])
(reg:VNx16BI 86 ffrt)) ... {aarch64_rdffr})
(insn 24 ...
(set (reg:VNx16BI 86 ffrt)
(unspec:VNx16BI [
(reg:VNx16BI 86 ffrt)
] UNSPEC_UPDATE_FFRT)) ... {aarch64_update_ffrt})
and so the problem is to eliminate the second load, it has to prove that
the write to the FFRT in insn 14 and insn 16 are the same which it can't.
Or did I misunderstand the case you were talking about?
Thanks,
Tamar
> Thoughts? Happy to take your solution if you’d still rather.
>
> KR,
> Alfie
>
> ________________________________
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: 07 August 2026 09:23
> To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>; ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org <iains@gcc.gnu.org>
> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> > -----Original Message-----
> > From: Alfie Richards <alfie.richards@arm.com>
> > Sent: 05 August 2026 16:42
> > To: gcc-patches@gcc.gnu.org
> > Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> > <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> > andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard Earnshaw
> > <Richard.Earnshaw@arm.com>; Tamar Christina <Tamar.Christina@arm.com>;
> > Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> > Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >
> > Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> > the FFR loads to the associated FFR updates.
> >
> > This isn't quite perfectly true, as the hardware presumably could set
> > return different FFR values from the exact same load happening twice, but that
> > is a nonsense situation where the optimisation is valid.
> >
>
> Hi Alfie,
>
> I think the idea is good, but I don't quite like that now every first faulting load shape
> needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
>
> If I’m not mistaken the problem is that CSE lib essentially sees
>
> (set (reg:VNx16BI FFR_REGNUM)
> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> (reg:VNx16BI FFR_REGNUM)]
> UNSPEC_UPDATE_FFR))
> (set (reg:VNx16BI FFR_REGNUM)
> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> (reg:VNx16BI FFR_REGNUM)]
> UNSPEC_UPDATE_FFR))
>
> And because these are pseudo registers and we only have one of them the REGNOs are the same
> and so it assumes they are the same. The patch fixes it by adding the load arguments to the
> UNSPEC_UPDATE_FFR to make them unique to the load.
>
> But I think we can fix this simpler by just adding a unique token to the aarch64_update_ffr_for_load.
>
> Like
>
> (define_insn "aarch64_update_ffr_for_load"
> [(set (reg:VNx16BI FFR_REGNUM)
> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> (reg:VNx16BI FFR_REGNUM)
> (match_operand 0 "const_int_operand" "n")]
> UNSPEC_UPDATE_FFR))]
>
> Then change
>
> emit_insn (gen_aarch64_update_ffr_for_load ());
>
> into
>
> emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
>
> the exact value of which doesn't really matter as long as it's unique (maybe make a helper).
>
> That should give us a unique aarch64_update_ffr_for_load and also like your patch still
> allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
>
> Thanks,
> Tamar
>
> > PR 126629
> >
> > gcc/ChangeLog:
> >
> > * config/aarch64/aarch64-sve-builtins-base.cc:
> > (svldff1_gather_impl::expand): Remove
> > gen_aarch64_update_ffr_for_load.
> > (svldff1_svldff1_gather_extend::expand): Ditto.
> > (svldxf1_impl::expand): Ditto.
> > (svldxf1_extend_impl::expand): Ditto.
> > * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> > Remove.
> > (*aarch64_update_ffr_for_continuous_load): New.
> > (*aarch64_update_ffr_for_gather_load): New.
> > (@aarch64_ld<fn>f1<mode>): New.
> > (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> > (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> > PARTIAL_I:mode>): New.
> > (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> > ARTIAL_I:mode>): Change to not be expanded directly.
> > (@aarch64_ldff1_gather<mode>): New.
> > (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> > (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> > <VNx4_NARROW:mode>): New.
> > (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> > VNx4_NARROW:mode>): Change to not be expanded directly.
> > (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> > <VNx2_NARROW:mode>): New.
> > (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> > VNx2_NARROW:mode>): Change to not be expanded directly.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/aarch64/sve/pr126629.c: New test.
> > * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> > * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> > * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
> >
> > -- >8 --
> >
> > Bootstrapped and regression tested on aarch64.
> >
> > Okay for master and backport?
> >
> > Thanks,
> > Alfie
> >
> > ---
> > .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> > gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> > .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> > .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> > .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> > .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> > 6 files changed, 279 insertions(+), 25 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > create mode 100644
> > gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > create mode 100644
> > gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > create mode 100644
> > gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >
> > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > index 7f047bb6468..186f223ac63 100644
> > --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > @@ -2001,7 +2001,6 @@ public:
> > {
> > /* See the block comment in aarch64-sve.md for details about the
> > FFR handling. */
> > - emit_insn (gen_aarch64_update_ffr_for_load ());
> >
> > e.prepare_gather_address_operands (1);
> > /* Put the predicate last, since ldff1_gather uses the same operand
> > @@ -2023,7 +2022,6 @@ public:
> > {
> > /* See the block comment in aarch64-sve.md for details about the
> > FFR handling. */
> > - emit_insn (gen_aarch64_update_ffr_for_load ());
> >
> > e.prepare_gather_address_operands (1);
> > /* Put the predicate last, since ldff1_gather uses the same operand
> > @@ -2075,7 +2073,6 @@ public:
> > {
> > /* See the block comment in aarch64-sve.md for details about the
> > FFR handling. */
> > - emit_insn (gen_aarch64_update_ffr_for_load ());
> >
> > machine_mode mode = e.vector_mode (0);
> > return e.use_contiguous_load_insn (code_for_aarch64_ldf1 (m_unspec,
> > mode));
> > @@ -2103,7 +2100,6 @@ public:
> > {
> > /* See the block comment in aarch64-sve.md for details about the
> > FFR handling. */
> > - emit_insn (gen_aarch64_update_ffr_for_load ());
> >
> > insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code (),
> > e.vector_mode (0),
> > diff --git a/gcc/config/aarch64/aarch64-sve.md
> > b/gcc/config/aarch64/aarch64-sve.md
> > index 105b34eb8fa..1e131cc404a 100644
> > --- a/gcc/config/aarch64/aarch64-sve.md
> > +++ b/gcc/config/aarch64/aarch64-sve.md
> > @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> > ;; so that the FFR value is live on entry to the region and so that the FFR
> > ;; value visibly changes within the region. This is used (possibly multiple
> > ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> > -(define_insn "aarch64_update_ffr_for_load"
> > +(define_insn "*aarch64_update_ffr_for_continuous_load"
> > [(set (reg:VNx16BI FFR_REGNUM)
> > - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > - (reg:VNx16BI FFR_REGNUM)]
> > UNSPEC_UPDATE_FFR))]
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_operand 0 "" "X")
> > + (match_operand 1 "" "X")
> > + ] UNSPEC_UPDATE_FFR))]
> > + "TARGET_SVE"
> > + ""
> > + [(set_attr "type" "no_insn")]
> > +)
> > +
> > +(define_insn "*aarch64_update_ffr_for_gather_load"
> > + [(set (reg:VNx16BI FFR_REGNUM)
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_operand 0 "" "X")
> > + (match_operand 1 "" "X")
> > + (match_operand 2 "" "X")
> > + (match_operand 3 "" "X")
> > + (match_operand 4 "" "X")
> > + ] UNSPEC_UPDATE_FFR)
> > + )]
> > "TARGET_SVE"
> > ""
> > [(set_attr "type" "no_insn")]
> > @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> > "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> > ;; -------------------------------------------------------------------------
> >
> > ;; Contiguous non-extending first-faulting or non-faulting loads.
> > -(define_insn "@aarch64_ld<fn>f1<mode>"
> > +(define_expand "@aarch64_ld<fn>f1<mode>"
> > + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_dup 2)
> > + (match_dup 1)
> > + ] UNSPEC_UPDATE_FFR)
> > + )
> > + (set (match_operand:SVE_FULL 0 "register_operand")
> > + (unspec:SVE_FULL
> > + [(match_operand:<VPRED> 2 "register_operand")
> > + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + SVE_LDFF1_LDNF1))]
> > + "TARGET_SVE && TARGET_NON_STREAMING"
> > + {}
> > +)
> > +
> > +(define_insn "*aarch64_ld<fn>f1<mode>"
> > [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> > (unspec:SVE_FULL
> > [(match_operand:<VPRED> 2 "register_operand" "Upl")
> > @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> > ;; -------------------------------------------------------------------------
> >
> > ;; Predicated first-faulting or non-faulting load and extend.
> > -(define_insn_and_rewrite
> > "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> > _I:mode>"
> > +(define_expand
> > "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> > _I:mode>"
> > + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_dup 2)
> > + (match_dup 1)
> > + ] UNSPEC_UPDATE_FFR)
> > + )
> > + (set (match_operand:SVE_HSDI 0 "register_operand")
> > + (unspec:SVE_HSDI
> > + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> > + (ANY_EXTEND:SVE_HSDI
> > + (unspec:SVE_PARTIAL_I
> > + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> > + (match_operand:SVE_PARTIAL_I 1
> > "aarch64_sve_ld<fn>f1_operand")
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + SVE_LDFF1_LDNF1))]
> > + UNSPEC_PRED_X))]
> > + "TARGET_SVE
> > + && TARGET_NON_STREAMING
> > + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> > + {
> > + if (!CONSTANT_P (operands[3]))
> > + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> > + }
> > +)
> > +
> > +;; Predicated first-faulting or non-faulting load and extend.
> > +(define_insn
> > "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> > I:mode>"
> > [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> > (unspec:SVE_HSDI
> > [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> > "UplDnm")
> > @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> > "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> > && TARGET_NON_STREAMING
> > && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> >
> > "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> > %2/z, %1"
> > - "&& !CONSTANT_P (operands[3])"
> > - {
> > - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> > - }
> > [(set_attr "sve_type" "sve_load_1reg")]
> > )
> >
> > @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> > "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
> >
> > ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> > ;; 3 is true for unsigned extension and false for signed extension.
> > -(define_insn "@aarch64_ldff1_gather<mode>"
> > +(define_expand "@aarch64_ldff1_gather<mode>"
> > + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_dup 5)
> > + (match_dup 1)
> > + (match_dup 2)
> > + (match_dup 3)
> > + (match_dup 4)
> > + ] UNSPEC_UPDATE_FFR)
> > + )
> > + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual load
> > + (unspec:SVE_FULL_S
> > + [(match_operand:VNx4BI 5 "register_operand")
> > + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> > + (match_operand:VNx4SI 2 "register_operand")
> > + (match_operand:DI 3 "const_int_operand")
> > + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> > + (mem:BLK (scratch))
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + UNSPEC_LDFF1_GATHER))]
> > + "TARGET_SVE && TARGET_NON_STREAMING"
> > + {}
> > +)
> > +
> > +(define_insn "*aarch64_ldff1_gather<mode>"
> > [(set (match_operand:SVE_FULL_S 0 "register_operand")
> > (unspec:SVE_FULL_S
> > [(match_operand:VNx4BI 5 "register_operand")
> > @@ -1938,7 +2029,33 @@ (define_insn "@aarch64_ldff1_gather<mode>"
> >
> > ;; Predicated first-faulting gather loads for 64-bit elements. The value
> > ;; of operand 3 doesn't matter in this case.
> > -(define_insn "@aarch64_ldff1_gather<mode>"
> > +(define_expand "@aarch64_ldff1_gather<mode>"
> > + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_dup 5)
> > + (match_dup 1)
> > + (match_dup 2)
> > + (match_dup 3)
> > + (match_dup 4)
> > + ] UNSPEC_UPDATE_FFR)
> > + )
> > + (set (match_operand:SVE_FULL_D 0 "register_operand")
> > + (unspec:SVE_FULL_D
> > + [(match_operand:VNx2BI 5 "register_operand")
> > + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> > + (match_operand:VNx2DI 2 "register_operand")
> > + (match_operand:DI 3 "const_int_operand")
> > + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> > + (mem:BLK (scratch))
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + UNSPEC_LDFF1_GATHER))]
> > + "TARGET_SVE && TARGET_NON_STREAMING"
> > + {}
> > +)
> > +
> > +(define_insn "*aarch64_ldff1_gather<mode>"
> > [(set (match_operand:SVE_FULL_D 0 "register_operand")
> > (unspec:SVE_FULL_D
> > [(match_operand:VNx2BI 5 "register_operand")
> > @@ -2032,7 +2149,40 @@ (define_insn
> > "*aarch64_ldff1_gather<mode>_uxtw"
> >
> > ;; Predicated extending first-faulting gather loads for 32-bit elements.
> > ;; Operand 3 is true for unsigned extension and false for signed extension.
> > -(define_insn_and_rewrite
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> > NARROW:mode>"
> > +(define_expand
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> > NARROW:mode>"
> > + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > + (unspec:VNx16BI
> > + [(reg:VNx16BI FFRT_REGNUM)
> > + (reg:VNx16BI FFR_REGNUM)
> > + (match_dup 5)
> > + (match_dup 1)
> > + (match_dup 2)
> > + (match_dup 3)
> > + (match_dup 4)
> > + ] UNSPEC_UPDATE_FFR)
> > + )
> > + (set (match_operand:VNx4_WIDE 0 "register_operand")
> > + (unspec:VNx4_WIDE
> > + [(match_operand:VNx4BI 6 "general_operand")
> > + (ANY_EXTEND:VNx4_WIDE
> > + (unspec:VNx4_NARROW
> > + [(match_operand:VNx4BI 5 "register_operand")
> > + (match_operand:DI 1
> > "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> > + (match_operand:VNx4_WIDE 2 "register_operand")
> > + (match_operand:DI 3 "const_int_operand")
> > + (match_operand:DI 4
> > "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> > + (mem:BLK (scratch))
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + UNSPEC_LDFF1_GATHER))]
> > + UNSPEC_PRED_X))]
> > + "TARGET_SVE && TARGET_NON_STREAMING"
> > + {
> > + if (!CONSTANT_P (operands[6]))
> > + operands[6] = CONSTM1_RTX (VNx4BImode);
> > + }
> > +)
> > +
> > +(define_insn
> > "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> > ARROW:mode>"
> > [(set (match_operand:VNx4_WIDE 0 "register_operand")
> > (unspec:VNx4_WIDE
> > [(match_operand:VNx4BI 6 "general_operand")
> > @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> > [&w, rk, w, Ui1, i, Upl, UplDnm]
> > ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s, uxtw
> > %p4]
> > [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> > }
> > - "&& !CONSTANT_P (operands[6])"
> > - {
> > - operands[6] = CONSTM1_RTX (VNx4BImode);
> > - }
> > [(set_attr "sve_type" "sve_gatherload_32")]
> > )
> >
> > ;; Predicated extending first-faulting gather loads for 64-bit elements.
> > ;; The value of operand 3 doesn't matter in this case.
> > -(define_insn_and_rewrite
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> > NARROW:mode>"
> > +(define_expand
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> > NARROW:mode>"
> > + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> > + (unspec:VNx2_WIDE
> > + [(match_operand:VNx2BI 6 "general_operand")
> > + (ANY_EXTEND:VNx2_WIDE
> > + (unspec:VNx2_NARROW
> > + [(match_operand:VNx2BI 5 "register_operand")
> > + (match_operand:DI 1
> > "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> > + (match_operand:VNx2_WIDE 2 "register_operand")
> > + (match_operand:DI 3 "const_int_operand")
> > + (match_operand:DI 4
> > "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> > + (mem:BLK (scratch))
> > + (reg:VNx16BI FFRT_REGNUM)]
> > + UNSPEC_LDFF1_GATHER))]
> > + UNSPEC_PRED_X))]
> > + "TARGET_SVE && TARGET_NON_STREAMING"
> > + {
> > + if (!CONSTANT_P (operands[6]))
> > + operands[6] = CONSTM1_RTX (VNx2BImode);
> > + }
> > +)
> > +
> > +(define_insn
> > "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> > ARROW:mode>"
> > [(set (match_operand:VNx2_WIDE 0 "register_operand")
> > (unspec:VNx2_WIDE
> > [(match_operand:VNx2BI 6 "general_operand")
> > @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> > [&w, rk, w, i, i, Upl, UplDnm]
> > ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d, lsl
> > %p4]
> > [?w, rk, w, i, i, Upl, UplDnm] ^
> > }
> > - "&& !CONSTANT_P (operands[6])"
> > - {
> > - operands[6] = CONSTM1_RTX (VNx2BImode);
> > - }
> > [(set_attr "sve_type" "sve_gatherload_64")]
> > )
> >
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > new file mode 100644
> > index 00000000000<tel:00000000000>..64cb3ece330
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > @@ -0,0 +1,23 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3" } */
> > +
> > +#include <arm_sve.h>
> > +
> > + void
> > +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> > +{
> > + svbool_t pt = svptrue_b8 ();
> > + svsetffr ();
> > + svuint8_t a = svldff1_u8 (pt, p);
> > + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > + svsetffr ();
> > + svuint8_t b = svldff1_u8 (pt, q);
> > + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > + svst1_u8 (pt, o1, a);
> > + svst1_u8 (pt, o2, b);
> > + *r1 = n1;
> > + *r2 = n2;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > new file mode 100644
> > index 00000000000<tel:00000000000>..909a9f1e651
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > @@ -0,0 +1,23 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3" } */
> > +
> > +#include <arm_sve.h>
> > +
> > + void
> > +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> > +{
> > + svbool_t pt = svptrue_b8 ();
> > + svsetffr ();
> > + svuint32_t a = svldff1ub_u32 (pt, p);
> > + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > + svsetffr ();
> > + svuint32_t b = svldff1ub_u32 (pt, q);
> > + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > + svst1_u32 (pt, o1, a);
> > + svst1_u32 (pt, o2, b);
> > + *r1 = n1;
> > + *r2 = n2;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > new file mode 100644
> > index 00000000000<tel:00000000000>..2348082fde5
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3" } */
> > +
> > +#include <arm_sve.h>
> > +
> > + void
> > +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > + unsigned long *r2, unsigned *o1, unsigned *o2,
> > + svuint32_t offset1, svuint32_t offset2)
> > +{
> > + svbool_t pt = svptrue_b8 ();
> > + svsetffr ();
> > + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> > + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > + svsetffr ();
> > + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> > + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > + svst1_u32 (pt, o1, a);
> > + svst1_u32 (pt, o2, b);
> > + *r1 = n1;
> > + *r2 = n2;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > new file mode 100644
> > index 00000000000<tel:00000000000>..bb9257fe382
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3" } */
> > +
> > +#include <arm_sve.h>
> > +
> > + void
> > +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> > + unsigned long *r2, unsigned *o1, unsigned *o2,
> > + long int offset1, long int offset2)
> > +{
> > + svbool_t pt = svptrue_b8 ();
> > + svsetffr ();
> > + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> > + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > + svsetffr ();
> > + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> > + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > + svst1_u32 (pt, o1, a);
> > + svst1_u32 (pt, o2, b);
> > + *r1 = n1;
> > + *r2 = n2;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > --
> > 2.34.1
>
--
On 07/08/2026 13:16, Tamar Christina wrote:
> Hi Alfie,
>
> The 08/07/2026 11:31, Alfie Richards wrote:
>> Hi Tamar,
>> Sorry replying from phone so poor formatting.
>>
>>> I think the idea is good, but I don't quite like that now every first faulting load shape
>>> needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
>>
>> I do agree this is the part of the patch I was most uncomfortable with.
>>
>>> And because these are pseudo registers and we only have one of them the REGNOs are the same
>>> and so it assumes they are the same. The patch fixes it by adding the load arguments to the
>>> UNSPEC_UPDATE_FFR to make them unique to the load.
>>
>> Essentially yes but it’s also because the whole FFR expression tree matches going back to the setffr instructions (I believe)
>>
>>> That should give us a unique aarch64_update_ffr_for_load and also like your patch still
>>> allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
>>
>> Yeah I like that idea, and I think it should work. I did think about something similar but my only hesitation was around the situation where you do two identical loads. I like in that situation with my patch that the duplicate load and rdffr gets removed whereas with the solution you propose we would remove the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big issue as it’s a pretty nonsense case but that’s why I went the way I did.
>>
>
> I'm not sure I understand which scenario this is.
>
> Applying your patch:
>
> #include <arm_sve.h>
>
> void
> same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> unsigned char *o2, unsigned long *r1,
> unsigned long *r2)
> {
> svbool_t pt = svptrue_b8 ();
>
> svsetffr ();
>
> svuint8_t a = svldff1_u8 (pt, p);
> unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>
> svuint8_t b = svldff1_u8 (pt, p);
> unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>
> svst1_u8 (pt, o1, a);
> svst1_u8 (pt, o2, b);
> *r1 = n1;
> *r2 = n2;
> }
Ah so it would need to be the same situation as the original case with
the double setffr, which is important to make the "expression" trees
identical. I guess this make this opimiation so restrictive to the point
of being even more negligable.
So:
#include <arm_sve.h>
void
same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
unsigned char *o2, unsigned long*r1,
unsigned long *r2)
{
svbool_t pt = svptrue_b8 ();
svsetffr ();
svuint8_t a = svldff1_u8 (pt, p);
unsigned long n1 = svcntp_b8 (pt, svrdffr ());
svsetffr ();
svuint8_t b = svldff1_u8 (pt, p);
unsigned long n2 = svcntp_b8 (pt, svrdffr ());
svst1_u8 (pt, o1, a);
svst1_u8 (pt, o2, b);
*r1 = n1;
*r2 = n2;
}
Gives:
same_load_read_same_load_read:
.LFB2:
.cfi_startproc
ptrue p7.b, all
setffr
ldff1b z31.b, p7/z, [x0]
st1b z31.b, p7, [x1]
rdffr p15.b
cntp x5, p7, p15.b
st1b z31.b, p7, [x2]
str x5, [x3]
str x5, [x4]
ret
.cfi_endproc
But again, pretty niche case.
Thanks,
Alfie
>
> Or did I misunderstand the case you were talking about?
>
> Thanks,
> Tamar
>
>> Thoughts? Happy to take your solution if you’d still rather.
>>
>> KR,
>> Alfie
>>
>> ________________________________
>> From: Tamar Christina <Tamar.Christina@arm.com>
>> Sent: 07 August 2026 09:23
>> To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
>> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>; ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org <iains@gcc.gnu.org>
>> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>>
>>> -----Original Message-----
>>> From: Alfie Richards <alfie.richards@arm.com>
>>> Sent: 05 August 2026 16:42
>>> To: gcc-patches@gcc.gnu.org
>>> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
>>> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
>>> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard Earnshaw
>>> <Richard.Earnshaw@arm.com>; Tamar Christina <Tamar.Christina@arm.com>;
>>> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
>>> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>>>
>>> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
>>> the FFR loads to the associated FFR updates.
>>>
>>> This isn't quite perfectly true, as the hardware presumably could set
>>> return different FFR values from the exact same load happening twice, but that
>>> is a nonsense situation where the optimisation is valid.
>>>
>>
>> Hi Alfie,
>>
>> I think the idea is good, but I don't quite like that now every first faulting load shape
>> needs Its own new update ffr and that the same UNSPEC has different amount of arguments.
>>
>> If I’m not mistaken the problem is that CSE lib essentially sees
>>
>> (set (reg:VNx16BI FFR_REGNUM)
>> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
>> (reg:VNx16BI FFR_REGNUM)]
>> UNSPEC_UPDATE_FFR))
>> (set (reg:VNx16BI FFR_REGNUM)
>> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
>> (reg:VNx16BI FFR_REGNUM)]
>> UNSPEC_UPDATE_FFR))
>>
>> And because these are pseudo registers and we only have one of them the REGNOs are the same
>> and so it assumes they are the same. The patch fixes it by adding the load arguments to the
>> UNSPEC_UPDATE_FFR to make them unique to the load.
>>
>> But I think we can fix this simpler by just adding a unique token to the aarch64_update_ffr_for_load.
>>
>> Like
>>
>> (define_insn "aarch64_update_ffr_for_load"
>> [(set (reg:VNx16BI FFR_REGNUM)
>> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
>> (reg:VNx16BI FFR_REGNUM)
>> (match_operand 0 "const_int_operand" "n")]
>> UNSPEC_UPDATE_FFR))]
>>
>> Then change
>>
>> emit_insn (gen_aarch64_update_ffr_for_load ());
>>
>> into
>>
>> emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
>>
>> the exact value of which doesn't really matter as long as it's unique (maybe make a helper).
>>
>> That should give us a unique aarch64_update_ffr_for_load and also like your patch still
>> allow removals of redundant setffrs and rdffrs but be a bit simpler and easier to backport.
>>
>> Thanks,
>> Tamar
>>
>>> PR 126629
>>>
>>> gcc/ChangeLog:
>>>
>>> * config/aarch64/aarch64-sve-builtins-base.cc:
>>> (svldff1_gather_impl::expand): Remove
>>> gen_aarch64_update_ffr_for_load.
>>> (svldff1_svldff1_gather_extend::expand): Ditto.
>>> (svldxf1_impl::expand): Ditto.
>>> (svldxf1_extend_impl::expand): Ditto.
>>> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
>>> Remove.
>>> (*aarch64_update_ffr_for_continuous_load): New.
>>> (*aarch64_update_ffr_for_gather_load): New.
>>> (@aarch64_ld<fn>f1<mode>): New.
>>> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
>>> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
>>> PARTIAL_I:mode>): New.
>>> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
>>> ARTIAL_I:mode>): Change to not be expanded directly.
>>> (@aarch64_ldff1_gather<mode>): New.
>>> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
>>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
>>> <VNx4_NARROW:mode>): New.
>>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
>>> VNx4_NARROW:mode>): Change to not be expanded directly.
>>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
>>> <VNx2_NARROW:mode>): New.
>>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
>>> VNx2_NARROW:mode>): Change to not be expanded directly.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/aarch64/sve/pr126629.c: New test.
>>> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
>>> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
>>> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
>>>
>>> -- >8 --
>>>
>>> Bootstrapped and regression tested on aarch64.
>>>
>>> Okay for master and backport?
>>>
>>> Thanks,
>>> Alfie
>>>
>>> ---
>>> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
>>> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
>>> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
>>> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
>>> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
>>> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
>>> 6 files changed, 279 insertions(+), 25 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
>>> create mode 100644
>>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
>>> create mode 100644
>>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
>>> create mode 100644
>>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>>>
>>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>>> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>>> index 7f047bb6468..186f223ac63 100644
>>> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>>> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
>>> @@ -2001,7 +2001,6 @@ public:
>>> {
>>> /* See the block comment in aarch64-sve.md for details about the
>>> FFR handling. */
>>> - emit_insn (gen_aarch64_update_ffr_for_load ());
>>>
>>> e.prepare_gather_address_operands (1);
>>> /* Put the predicate last, since ldff1_gather uses the same operand
>>> @@ -2023,7 +2022,6 @@ public:
>>> {
>>> /* See the block comment in aarch64-sve.md for details about the
>>> FFR handling. */
>>> - emit_insn (gen_aarch64_update_ffr_for_load ());
>>>
>>> e.prepare_gather_address_operands (1);
>>> /* Put the predicate last, since ldff1_gather uses the same operand
>>> @@ -2075,7 +2073,6 @@ public:
>>> {
>>> /* See the block comment in aarch64-sve.md for details about the
>>> FFR handling. */
>>> - emit_insn (gen_aarch64_update_ffr_for_load ());
>>>
>>> machine_mode mode = e.vector_mode (0);
>>> return e.use_contiguous_load_insn (code_for_aarch64_ldf1 (m_unspec,
>>> mode));
>>> @@ -2103,7 +2100,6 @@ public:
>>> {
>>> /* See the block comment in aarch64-sve.md for details about the
>>> FFR handling. */
>>> - emit_insn (gen_aarch64_update_ffr_for_load ());
>>>
>>> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code (),
>>> e.vector_mode (0),
>>> diff --git a/gcc/config/aarch64/aarch64-sve.md
>>> b/gcc/config/aarch64/aarch64-sve.md
>>> index 105b34eb8fa..1e131cc404a 100644
>>> --- a/gcc/config/aarch64/aarch64-sve.md
>>> +++ b/gcc/config/aarch64/aarch64-sve.md
>>> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
>>> ;; so that the FFR value is live on entry to the region and so that the FFR
>>> ;; value visibly changes within the region. This is used (possibly multiple
>>> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
>>> -(define_insn "aarch64_update_ffr_for_load"
>>> +(define_insn "*aarch64_update_ffr_for_continuous_load"
>>> [(set (reg:VNx16BI FFR_REGNUM)
>>> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
>>> - (reg:VNx16BI FFR_REGNUM)]
>>> UNSPEC_UPDATE_FFR))]
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_operand 0 "" "X")
>>> + (match_operand 1 "" "X")
>>> + ] UNSPEC_UPDATE_FFR))]
>>> + "TARGET_SVE"
>>> + ""
>>> + [(set_attr "type" "no_insn")]
>>> +)
>>> +
>>> +(define_insn "*aarch64_update_ffr_for_gather_load"
>>> + [(set (reg:VNx16BI FFR_REGNUM)
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_operand 0 "" "X")
>>> + (match_operand 1 "" "X")
>>> + (match_operand 2 "" "X")
>>> + (match_operand 3 "" "X")
>>> + (match_operand 4 "" "X")
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )]
>>> "TARGET_SVE"
>>> ""
>>> [(set_attr "type" "no_insn")]
>>> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
>>> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
>>> ;; -------------------------------------------------------------------------
>>>
>>> ;; Contiguous non-extending first-faulting or non-faulting loads.
>>> -(define_insn "@aarch64_ld<fn>f1<mode>"
>>> +(define_expand "@aarch64_ld<fn>f1<mode>"
>>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_dup 2)
>>> + (match_dup 1)
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )
>>> + (set (match_operand:SVE_FULL 0 "register_operand")
>>> + (unspec:SVE_FULL
>>> + [(match_operand:<VPRED> 2 "register_operand")
>>> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + SVE_LDFF1_LDNF1))]
>>> + "TARGET_SVE && TARGET_NON_STREAMING"
>>> + {}
>>> +)
>>> +
>>> +(define_insn "*aarch64_ld<fn>f1<mode>"
>>> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
>>> (unspec:SVE_FULL
>>> [(match_operand:<VPRED> 2 "register_operand" "Upl")
>>> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
>>> ;; -------------------------------------------------------------------------
>>>
>>> ;; Predicated first-faulting or non-faulting load and extend.
>>> -(define_insn_and_rewrite
>>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
>>> _I:mode>"
>>> +(define_expand
>>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
>>> _I:mode>"
>>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_dup 2)
>>> + (match_dup 1)
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )
>>> + (set (match_operand:SVE_HSDI 0 "register_operand")
>>> + (unspec:SVE_HSDI
>>> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
>>> + (ANY_EXTEND:SVE_HSDI
>>> + (unspec:SVE_PARTIAL_I
>>> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
>>> + (match_operand:SVE_PARTIAL_I 1
>>> "aarch64_sve_ld<fn>f1_operand")
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + SVE_LDFF1_LDNF1))]
>>> + UNSPEC_PRED_X))]
>>> + "TARGET_SVE
>>> + && TARGET_NON_STREAMING
>>> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
>>> + {
>>> + if (!CONSTANT_P (operands[3]))
>>> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
>>> + }
>>> +)
>>> +
>>> +;; Predicated first-faulting or non-faulting load and extend.
>>> +(define_insn
>>> "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
>>> I:mode>"
>>> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
>>> (unspec:SVE_HSDI
>>> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
>>> "UplDnm")
>>> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
>>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
>>> && TARGET_NON_STREAMING
>>> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
>>>
>>> "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
>>> %2/z, %1"
>>> - "&& !CONSTANT_P (operands[3])"
>>> - {
>>> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
>>> - }
>>> [(set_attr "sve_type" "sve_load_1reg")]
>>> )
>>>
>>> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
>>> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
>>>
>>> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
>>> ;; 3 is true for unsigned extension and false for signed extension.
>>> -(define_insn "@aarch64_ldff1_gather<mode>"
>>> +(define_expand "@aarch64_ldff1_gather<mode>"
>>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_dup 5)
>>> + (match_dup 1)
>>> + (match_dup 2)
>>> + (match_dup 3)
>>> + (match_dup 4)
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )
>>> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual load
>>> + (unspec:SVE_FULL_S
>>> + [(match_operand:VNx4BI 5 "register_operand")
>>> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
>>> + (match_operand:VNx4SI 2 "register_operand")
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
>>> + (mem:BLK (scratch))
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + UNSPEC_LDFF1_GATHER))]
>>> + "TARGET_SVE && TARGET_NON_STREAMING"
>>> + {}
>>> +)
>>> +
>>> +(define_insn "*aarch64_ldff1_gather<mode>"
>>> [(set (match_operand:SVE_FULL_S 0 "register_operand")
>>> (unspec:SVE_FULL_S
>>> [(match_operand:VNx4BI 5 "register_operand")
>>> @@ -1938,7 +2029,33 @@ (define_insn "@aarch64_ldff1_gather<mode>"
>>>
>>> ;; Predicated first-faulting gather loads for 64-bit elements. The value
>>> ;; of operand 3 doesn't matter in this case.
>>> -(define_insn "@aarch64_ldff1_gather<mode>"
>>> +(define_expand "@aarch64_ldff1_gather<mode>"
>>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_dup 5)
>>> + (match_dup 1)
>>> + (match_dup 2)
>>> + (match_dup 3)
>>> + (match_dup 4)
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )
>>> + (set (match_operand:SVE_FULL_D 0 "register_operand")
>>> + (unspec:SVE_FULL_D
>>> + [(match_operand:VNx2BI 5 "register_operand")
>>> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
>>> + (match_operand:VNx2DI 2 "register_operand")
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
>>> + (mem:BLK (scratch))
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + UNSPEC_LDFF1_GATHER))]
>>> + "TARGET_SVE && TARGET_NON_STREAMING"
>>> + {}
>>> +)
>>> +
>>> +(define_insn "*aarch64_ldff1_gather<mode>"
>>> [(set (match_operand:SVE_FULL_D 0 "register_operand")
>>> (unspec:SVE_FULL_D
>>> [(match_operand:VNx2BI 5 "register_operand")
>>> @@ -2032,7 +2149,40 @@ (define_insn
>>> "*aarch64_ldff1_gather<mode>_uxtw"
>>>
>>> ;; Predicated extending first-faulting gather loads for 32-bit elements.
>>> ;; Operand 3 is true for unsigned extension and false for signed extension.
>>> -(define_insn_and_rewrite
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
>>> NARROW:mode>"
>>> +(define_expand
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
>>> NARROW:mode>"
>>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
>>> + (unspec:VNx16BI
>>> + [(reg:VNx16BI FFRT_REGNUM)
>>> + (reg:VNx16BI FFR_REGNUM)
>>> + (match_dup 5)
>>> + (match_dup 1)
>>> + (match_dup 2)
>>> + (match_dup 3)
>>> + (match_dup 4)
>>> + ] UNSPEC_UPDATE_FFR)
>>> + )
>>> + (set (match_operand:VNx4_WIDE 0 "register_operand")
>>> + (unspec:VNx4_WIDE
>>> + [(match_operand:VNx4BI 6 "general_operand")
>>> + (ANY_EXTEND:VNx4_WIDE
>>> + (unspec:VNx4_NARROW
>>> + [(match_operand:VNx4BI 5 "register_operand")
>>> + (match_operand:DI 1
>>> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
>>> + (match_operand:VNx4_WIDE 2 "register_operand")
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (match_operand:DI 4
>>> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
>>> + (mem:BLK (scratch))
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + UNSPEC_LDFF1_GATHER))]
>>> + UNSPEC_PRED_X))]
>>> + "TARGET_SVE && TARGET_NON_STREAMING"
>>> + {
>>> + if (!CONSTANT_P (operands[6]))
>>> + operands[6] = CONSTM1_RTX (VNx4BImode);
>>> + }
>>> +)
>>> +
>>> +(define_insn
>>> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
>>> ARROW:mode>"
>>> [(set (match_operand:VNx4_WIDE 0 "register_operand")
>>> (unspec:VNx4_WIDE
>>> [(match_operand:VNx4BI 6 "general_operand")
>>> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
>>> [&w, rk, w, Ui1, i, Upl, UplDnm]
>>> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s, uxtw
>>> %p4]
>>> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
>>> }
>>> - "&& !CONSTANT_P (operands[6])"
>>> - {
>>> - operands[6] = CONSTM1_RTX (VNx4BImode);
>>> - }
>>> [(set_attr "sve_type" "sve_gatherload_32")]
>>> )
>>>
>>> ;; Predicated extending first-faulting gather loads for 64-bit elements.
>>> ;; The value of operand 3 doesn't matter in this case.
>>> -(define_insn_and_rewrite
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
>>> NARROW:mode>"
>>> +(define_expand
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
>>> NARROW:mode>"
>>> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
>>> + (unspec:VNx2_WIDE
>>> + [(match_operand:VNx2BI 6 "general_operand")
>>> + (ANY_EXTEND:VNx2_WIDE
>>> + (unspec:VNx2_NARROW
>>> + [(match_operand:VNx2BI 5 "register_operand")
>>> + (match_operand:DI 1
>>> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
>>> + (match_operand:VNx2_WIDE 2 "register_operand")
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (match_operand:DI 4
>>> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
>>> + (mem:BLK (scratch))
>>> + (reg:VNx16BI FFRT_REGNUM)]
>>> + UNSPEC_LDFF1_GATHER))]
>>> + UNSPEC_PRED_X))]
>>> + "TARGET_SVE && TARGET_NON_STREAMING"
>>> + {
>>> + if (!CONSTANT_P (operands[6]))
>>> + operands[6] = CONSTM1_RTX (VNx2BImode);
>>> + }
>>> +)
>>> +
>>> +(define_insn
>>> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
>>> ARROW:mode>"
>>> [(set (match_operand:VNx2_WIDE 0 "register_operand")
>>> (unspec:VNx2_WIDE
>>> [(match_operand:VNx2BI 6 "general_operand")
>>> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
>>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
>>> [&w, rk, w, i, i, Upl, UplDnm]
>>> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d, lsl
>>> %p4]
>>> [?w, rk, w, i, i, Upl, UplDnm] ^
>>> }
>>> - "&& !CONSTANT_P (operands[6])"
>>> - {
>>> - operands[6] = CONSTM1_RTX (VNx2BImode);
>>> - }
>>> [(set_attr "sve_type" "sve_gatherload_64")]
>>> )
>>>
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
>>> new file mode 100644
>>> index 00000000000<tel:00000000000>..64cb3ece330
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
>>> @@ -0,0 +1,23 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#include <arm_sve.h>
>>> +
>>> + void
>>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
>>> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
>>> +{
>>> + svbool_t pt = svptrue_b8 ();
>>> + svsetffr ();
>>> + svuint8_t a = svldff1_u8 (pt, p);
>>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>>> + svsetffr ();
>>> + svuint8_t b = svldff1_u8 (pt, q);
>>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>>> + svst1_u8 (pt, o1, a);
>>> + svst1_u8 (pt, o2, b);
>>> + *r1 = n1;
>>> + *r2 = n2;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
>>> new file mode 100644
>>> index 00000000000<tel:00000000000>..909a9f1e651
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
>>> @@ -0,0 +1,23 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#include <arm_sve.h>
>>> +
>>> + void
>>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
>>> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
>>> +{
>>> + svbool_t pt = svptrue_b8 ();
>>> + svsetffr ();
>>> + svuint32_t a = svldff1ub_u32 (pt, p);
>>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>>> + svsetffr ();
>>> + svuint32_t b = svldff1ub_u32 (pt, q);
>>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>>> + svst1_u32 (pt, o1, a);
>>> + svst1_u32 (pt, o2, b);
>>> + *r1 = n1;
>>> + *r2 = n2;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
>>> new file mode 100644
>>> index 00000000000<tel:00000000000>..2348082fde5
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
>>> @@ -0,0 +1,24 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#include <arm_sve.h>
>>> +
>>> + void
>>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
>>> + unsigned long *r2, unsigned *o1, unsigned *o2,
>>> + svuint32_t offset1, svuint32_t offset2)
>>> +{
>>> + svbool_t pt = svptrue_b8 ();
>>> + svsetffr ();
>>> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
>>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>>> + svsetffr ();
>>> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
>>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>>> + svst1_u32 (pt, o1, a);
>>> + svst1_u32 (pt, o2, b);
>>> + *r1 = n1;
>>> + *r2 = n2;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>>> new file mode 100644
>>> index 00000000000<tel:00000000000>..bb9257fe382
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
>>> @@ -0,0 +1,24 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#include <arm_sve.h>
>>> +
>>> + void
>>> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
>>> + unsigned long *r2, unsigned *o1, unsigned *o2,
>>> + long int offset1, long int offset2)
>>> +{
>>> + svbool_t pt = svptrue_b8 ();
>>> + svsetffr ();
>>> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
>>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>>> + svsetffr ();
>>> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
>>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>>> + svst1_u32 (pt, o1, a);
>>> + svst1_u32 (pt, o2, b);
>>> + *r1 = n1;
>>> + *r2 = n2;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
>>> --
>>> 2.34.1
>>
>
Hi Alfie,
> -----Original Message-----
> From: Alfie Richards <Alfie.Richards@arm.com>
> Sent: 07 August 2026 14:05
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Alex Coplan <Alex.Coplan@arm.com>; Alice
> Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com;
> ktkachov@nvidia.com; Richard Earnshaw <Richard.Earnshaw@arm.com>;
> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> Subject: Re: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> On 07/08/2026 13:16, Tamar Christina wrote:
> > Hi Alfie,
> >
> > The 08/07/2026 11:31, Alfie Richards wrote:
> >> Hi Tamar,
> >> Sorry replying from phone so poor formatting.
> >>
> >>> I think the idea is good, but I don't quite like that now every first faulting
> load shape
> >>> needs Its own new update ffr and that the same UNSPEC has different
> amount of arguments.
> >>
> >> I do agree this is the part of the patch I was most uncomfortable with.
> >>
> >>> And because these are pseudo registers and we only have one of them the
> REGNOs are the same
> >>> and so it assumes they are the same. The patch fixes it by adding the load
> arguments to the
> >>> UNSPEC_UPDATE_FFR to make them unique to the load.
> >>
> >> Essentially yes but it’s also because the whole FFR expression tree matches
> going back to the setffr instructions (I believe)
> >>
> >>> That should give us a unique aarch64_update_ffr_for_load and also like
> your patch still
> >>> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> easier to backport.
> >>
> >> Yeah I like that idea, and I think it should work. I did think about something
> similar but my only hesitation was around the situation where you do two
> identical loads. I like in that situation with my patch that the duplicate load and
> rdffr gets removed whereas with the solution you propose we would remove
> the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big
> issue as it’s a pretty nonsense case but that’s why I went the way I did.
> >>
> >
> > I'm not sure I understand which scenario this is.
> >
> > Applying your patch:
> >
> > #include <arm_sve.h>
> >
> > void
> > same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> > unsigned char *o2, unsigned long *r1,
> > unsigned long *r2)
> > {
> > svbool_t pt = svptrue_b8 ();
> >
> > svsetffr ();
> >
> > svuint8_t a = svldff1_u8 (pt, p);
> > unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >
> > svuint8_t b = svldff1_u8 (pt, p);
> > unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >
> > svst1_u8 (pt, o1, a);
> > svst1_u8 (pt, o2, b);
> > *r1 = n1;
> > *r2 = n2;
> > }
>
> Ah so it would need to be the same situation as the original case with
> the double setffr, which is important to make the "expression" trees
> identical. I guess this make this opimiation so restrictive to the point
> of being even more negligable.
>
> So:
>
> #include <arm_sve.h>
>
> void
> same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> unsigned char *o2, unsigned long*r1,
> unsigned long *r2)
> {
> svbool_t pt = svptrue_b8 ();
>
> svsetffr ();
>
> svuint8_t a = svldff1_u8 (pt, p);
> unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>
> svsetffr ();
>
> svuint8_t b = svldff1_u8 (pt, p);
> unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>
> svst1_u8 (pt, o1, a);
> svst1_u8 (pt, o2, b);
> *r1 = n1;
> *r2 = n2;
> }
>
> Gives:
>
> same_load_read_same_load_read:
> .LFB2:
> .cfi_startproc
> ptrue p7.b, all
> setffr
> ldff1b z31.b, p7/z, [x0]
> st1b z31.b, p7, [x1]
> rdffr p15.b
> cntp x5, p7, p15.b
> st1b z31.b, p7, [x2]
> str x5, [x3]
> str x5, [x4]
> ret
> .cfi_endproc
>
> But again, pretty niche case.
For that all you need is to use the value result of the load
as the unique token. So instead of const_int you have
(define_insn "aarch64_update_ffr_for_load"
[(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)
(match_operand 0 "register_operand" "w")]
UNSPEC_UPDATE_FFR))]
And then the emitters become
rtx res = e.use_contiguous_load_insn (icode);
emit_insn (gen_aarch64_update_ffr_for_load (res));
return res;
etc. When the optimizers prove the loads are the same the results
become the same pseudos or get an equivalent relationship set which
makes the UNSPEC_UPDATE_FFRs the same and so they can be CSEd.
Thanks,
Tamar
>
> Thanks,
> Alfie
> >
> > Or did I misunderstand the case you were talking about?
> >
> > Thanks,
> > Tamar
> >
> >> Thoughts? Happy to take your solution if you’d still rather.
> >>
> >> KR,
> >> Alfie
> >>
> >> ________________________________
> >> From: Tamar Christina <Tamar.Christina@arm.com>
> >> Sent: 07 August 2026 09:23
> >> To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org
> <gcc-patches@gcc.gnu.org>
> >> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>;
> ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>;
> iains@gcc.gnu.org <iains@gcc.gnu.org>
> >> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >>
> >>> -----Original Message-----
> >>> From: Alfie Richards <alfie.richards@arm.com>
> >>> Sent: 05 August 2026 16:42
> >>> To: gcc-patches@gcc.gnu.org
> >>> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> >>> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> >>> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard
> Earnshaw
> >>> <Richard.Earnshaw@arm.com>; Tamar Christina
> <Tamar.Christina@arm.com>;
> >>> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> >>> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >>>
> >>> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> >>> the FFR loads to the associated FFR updates.
> >>>
> >>> This isn't quite perfectly true, as the hardware presumably could set
> >>> return different FFR values from the exact same load happening twice, but
> that
> >>> is a nonsense situation where the optimisation is valid.
> >>>
> >>
> >> Hi Alfie,
> >>
> >> I think the idea is good, but I don't quite like that now every first faulting
> load shape
> >> needs Its own new update ffr and that the same UNSPEC has different
> amount of arguments.
> >>
> >> If I’m not mistaken the problem is that CSE lib essentially sees
> >>
> >> (set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)]
> >> UNSPEC_UPDATE_FFR))
> >> (set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)]
> >> UNSPEC_UPDATE_FFR))
> >>
> >> And because these are pseudo registers and we only have one of them the
> REGNOs are the same
> >> and so it assumes they are the same. The patch fixes it by adding the load
> arguments to the
> >> UNSPEC_UPDATE_FFR to make them unique to the load.
> >>
> >> But I think we can fix this simpler by just adding a unique token to the
> aarch64_update_ffr_for_load.
> >>
> >> Like
> >>
> >> (define_insn "aarch64_update_ffr_for_load"
> >> [(set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)
> >> (match_operand 0 "const_int_operand" "n")]
> >> UNSPEC_UPDATE_FFR))]
> >>
> >> Then change
> >>
> >> emit_insn (gen_aarch64_update_ffr_for_load ());
> >>
> >> into
> >>
> >> emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
> >>
> >> the exact value of which doesn't really matter as long as it's unique (maybe
> make a helper).
> >>
> >> That should give us a unique aarch64_update_ffr_for_load and also like
> your patch still
> >> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> easier to backport.
> >>
> >> Thanks,
> >> Tamar
> >>
> >>> PR 126629
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * config/aarch64/aarch64-sve-builtins-base.cc:
> >>> (svldff1_gather_impl::expand): Remove
> >>> gen_aarch64_update_ffr_for_load.
> >>> (svldff1_svldff1_gather_extend::expand): Ditto.
> >>> (svldxf1_impl::expand): Ditto.
> >>> (svldxf1_extend_impl::expand): Ditto.
> >>> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> >>> Remove.
> >>> (*aarch64_update_ffr_for_continuous_load): New.
> >>> (*aarch64_update_ffr_for_gather_load): New.
> >>> (@aarch64_ld<fn>f1<mode>): New.
> >>> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> >>> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> >>> PARTIAL_I:mode>): New.
> >>> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> >>> ARTIAL_I:mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather<mode>): New.
> >>> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> >>> <VNx4_NARROW:mode>): New.
> >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> >>> VNx4_NARROW:mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> >>> <VNx2_NARROW:mode>): New.
> >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> >>> VNx2_NARROW:mode>): Change to not be expanded directly.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>> * gcc.target/aarch64/sve/pr126629.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
> >>>
> >>> -- >8 --
> >>>
> >>> Bootstrapped and regression tested on aarch64.
> >>>
> >>> Okay for master and backport?
> >>>
> >>> Thanks,
> >>> Alfie
> >>>
> >>> ---
> >>> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> >>> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> >>> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> >>> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> >>> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> >>> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> >>> 6 files changed, 279 insertions(+), 25 deletions(-)
> >>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>>
> >>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> index 7f047bb6468..186f223ac63 100644
> >>> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> @@ -2001,7 +2001,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> e.prepare_gather_address_operands (1);
> >>> /* Put the predicate last, since ldff1_gather uses the same operand
> >>> @@ -2023,7 +2022,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> e.prepare_gather_address_operands (1);
> >>> /* Put the predicate last, since ldff1_gather uses the same operand
> >>> @@ -2075,7 +2073,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> machine_mode mode = e.vector_mode (0);
> >>> return e.use_contiguous_load_insn (code_for_aarch64_ldf1
> (m_unspec,
> >>> mode));
> >>> @@ -2103,7 +2100,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code
> (),
> >>> e.vector_mode (0),
> >>> diff --git a/gcc/config/aarch64/aarch64-sve.md
> >>> b/gcc/config/aarch64/aarch64-sve.md
> >>> index 105b34eb8fa..1e131cc404a 100644
> >>> --- a/gcc/config/aarch64/aarch64-sve.md
> >>> +++ b/gcc/config/aarch64/aarch64-sve.md
> >>> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> >>> ;; so that the FFR value is live on entry to the region and so that the FFR
> >>> ;; value visibly changes within the region. This is used (possibly multiple
> >>> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> >>> -(define_insn "aarch64_update_ffr_for_load"
> >>> +(define_insn "*aarch64_update_ffr_for_continuous_load"
> >>> [(set (reg:VNx16BI FFR_REGNUM)
> >>> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >>> - (reg:VNx16BI FFR_REGNUM)]
> >>> UNSPEC_UPDATE_FFR))]
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_operand 0 "" "X")
> >>> + (match_operand 1 "" "X")
> >>> + ] UNSPEC_UPDATE_FFR))]
> >>> + "TARGET_SVE"
> >>> + ""
> >>> + [(set_attr "type" "no_insn")]
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_update_ffr_for_gather_load"
> >>> + [(set (reg:VNx16BI FFR_REGNUM)
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_operand 0 "" "X")
> >>> + (match_operand 1 "" "X")
> >>> + (match_operand 2 "" "X")
> >>> + (match_operand 3 "" "X")
> >>> + (match_operand 4 "" "X")
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )]
> >>> "TARGET_SVE"
> >>> ""
> >>> [(set_attr "type" "no_insn")]
> >>> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> >>> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> >>> ;; -------------------------------------------------------------------------
> >>>
> >>> ;; Contiguous non-extending first-faulting or non-faulting loads.
> >>> -(define_insn "@aarch64_ld<fn>f1<mode>"
> >>> +(define_expand "@aarch64_ld<fn>f1<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 2)
> >>> + (match_dup 1)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL 0 "register_operand")
> >>> + (unspec:SVE_FULL
> >>> + [(match_operand:<VPRED> 2 "register_operand")
> >>> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + SVE_LDFF1_LDNF1))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ld<fn>f1<mode>"
> >>> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> >>> (unspec:SVE_FULL
> >>> [(match_operand:<VPRED> 2 "register_operand" "Upl")
> >>> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> >>> ;; -------------------------------------------------------------------------
> >>>
> >>> ;; Predicated first-faulting or non-faulting load and extend.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> >>> _I:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> >>> _I:mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 2)
> >>> + (match_dup 1)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_HSDI 0 "register_operand")
> >>> + (unspec:SVE_HSDI
> >>> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> >>> + (ANY_EXTEND:SVE_HSDI
> >>> + (unspec:SVE_PARTIAL_I
> >>> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> >>> + (match_operand:SVE_PARTIAL_I 1
> >>> "aarch64_sve_ld<fn>f1_operand")
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + SVE_LDFF1_LDNF1))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE
> >>> + && TARGET_NON_STREAMING
> >>> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) ==
> 0"
> >>> + {
> >>> + if (!CONSTANT_P (operands[3]))
> >>> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> >>> + }
> >>> +)
> >>> +
> >>> +;; Predicated first-faulting or non-faulting load and extend.
> >>> +(define_insn
> >>>
> "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> >>> I:mode>"
> >>> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> >>> (unspec:SVE_HSDI
> >>> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> >>> "UplDnm")
> >>> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> >>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> >>> && TARGET_NON_STREAMING
> >>> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> >>>
> >>>
> "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> >>> %2/z, %1"
> >>> - "&& !CONSTANT_P (operands[3])"
> >>> - {
> >>> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_load_1reg")]
> >>> )
> >>>
> >>> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> >>> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
> >>>
> >>> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> >>> ;; 3 is true for unsigned extension and false for signed extension.
> >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual
> load
> >>> + (unspec:SVE_FULL_S
> >>> + [(match_operand:VNx4BI 5 "register_operand")
> >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> >>> + (match_operand:VNx4SI 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> >>> [(set (match_operand:SVE_FULL_S 0 "register_operand")
> >>> (unspec:SVE_FULL_S
> >>> [(match_operand:VNx4BI 5 "register_operand")
> >>> @@ -1938,7 +2029,33 @@ (define_insn
> "@aarch64_ldff1_gather<mode>"
> >>>
> >>> ;; Predicated first-faulting gather loads for 64-bit elements. The value
> >>> ;; of operand 3 doesn't matter in this case.
> >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL_D 0 "register_operand")
> >>> + (unspec:SVE_FULL_D
> >>> + [(match_operand:VNx2BI 5 "register_operand")
> >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> >>> + (match_operand:VNx2DI 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> >>> [(set (match_operand:SVE_FULL_D 0 "register_operand")
> >>> (unspec:SVE_FULL_D
> >>> [(match_operand:VNx2BI 5 "register_operand")
> >>> @@ -2032,7 +2149,40 @@ (define_insn
> >>> "*aarch64_ldff1_gather<mode>_uxtw"
> >>>
> >>> ;; Predicated extending first-faulting gather loads for 32-bit elements.
> >>> ;; Operand 3 is true for unsigned extension and false for signed extension.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> >>> NARROW:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> >>> NARROW:mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:VNx4_WIDE 0 "register_operand")
> >>> + (unspec:VNx4_WIDE
> >>> + [(match_operand:VNx4BI 6 "general_operand")
> >>> + (ANY_EXTEND:VNx4_WIDE
> >>> + (unspec:VNx4_NARROW
> >>> + [(match_operand:VNx4BI 5 "register_operand")
> >>> + (match_operand:DI 1
> >>> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> >>> + (match_operand:VNx4_WIDE 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4
> >>> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {
> >>> + if (!CONSTANT_P (operands[6]))
> >>> + operands[6] = CONSTM1_RTX (VNx4BImode);
> >>> + }
> >>> +)
> >>> +
> >>> +(define_insn
> >>>
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> >>> ARROW:mode>"
> >>> [(set (match_operand:VNx4_WIDE 0 "register_operand")
> >>> (unspec:VNx4_WIDE
> >>> [(match_operand:VNx4BI 6 "general_operand")
> >>> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> >>> [&w, rk, w, Ui1, i, Upl, UplDnm]
> >>> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s,
> uxtw
> >>> %p4]
> >>> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> >>> }
> >>> - "&& !CONSTANT_P (operands[6])"
> >>> - {
> >>> - operands[6] = CONSTM1_RTX (VNx4BImode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_gatherload_32")]
> >>> )
> >>>
> >>> ;; Predicated extending first-faulting gather loads for 64-bit elements.
> >>> ;; The value of operand 3 doesn't matter in this case.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> >>> NARROW:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> >>> NARROW:mode>"
> >>> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> >>> + (unspec:VNx2_WIDE
> >>> + [(match_operand:VNx2BI 6 "general_operand")
> >>> + (ANY_EXTEND:VNx2_WIDE
> >>> + (unspec:VNx2_NARROW
> >>> + [(match_operand:VNx2BI 5 "register_operand")
> >>> + (match_operand:DI 1
> >>> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> >>> + (match_operand:VNx2_WIDE 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4
> >>> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {
> >>> + if (!CONSTANT_P (operands[6]))
> >>> + operands[6] = CONSTM1_RTX (VNx2BImode);
> >>> + }
> >>> +)
> >>> +
> >>> +(define_insn
> >>>
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> >>> ARROW:mode>"
> >>> [(set (match_operand:VNx2_WIDE 0 "register_operand")
> >>> (unspec:VNx2_WIDE
> >>> [(match_operand:VNx2BI 6 "general_operand")
> >>> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> >>> [&w, rk, w, i, i, Upl, UplDnm]
> >>> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d,
> lsl
> >>> %p4]
> >>> [?w, rk, w, i, i, Upl, UplDnm] ^
> >>> }
> >>> - "&& !CONSTANT_P (operands[6])"
> >>> - {
> >>> - operands[6] = CONSTM1_RTX (VNx2BImode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_gatherload_64")]
> >>> )
> >>>
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000>..64cb3ece330
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> @@ -0,0 +1,23 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint8_t a = svldff1_u8 (pt, p);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint8_t b = svldff1_u8 (pt, q);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u8 (pt, o1, a);
> >>> + svst1_u8 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000>..909a9f1e651
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> @@ -0,0 +1,23 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1ub_u32 (pt, p);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1ub_u32 (pt, q);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000>..2348082fde5
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> @@ -0,0 +1,24 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> >>> + svuint32_t offset1, svuint32_t offset2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000>..bb9257fe382
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> @@ -0,0 +1,24 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> >>> + long int offset1, long int offset2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> --
> >>> 2.34.1
> >>
> >
Hi Tamar,
(Sorry on my phone again)
> For that all you need is to use the value result of the load
> as the unique token. So instead of const_int you have
> (define_insn "aarch64_update_ffr_for_load"
> [(set (reg:VNx16BI FFR_REGNUM)
> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> (reg:VNx16BI FFR_REGNUM)
> (match_operand 0 "register_operand" "w")]
> UNSPEC_UPDATE_FFR))]
I did consider this too but my concern then is that you force the load values live even if the results aren’t used. (Provided the ffr stays live which it often does) So we may lose DCE on the load.
Sorry to be so picky about this, I’m aware it doesn’t really matter that much.
I think in balance the max_uid solution may be best? Happy to re-spin with that when I’m back.
Thanks,
Alfie
________________________________
From: Tamar Christina <Tamar.Christina@arm.com>
Sent: 10 August 2026 08:10
To: Alfie Richards <Alfie.Richards@arm.com>
Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; Alex Coplan <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>; ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org <iains@gcc.gnu.org>
Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
Hi Alfie,
> -----Original Message-----
> From: Alfie Richards <Alfie.Richards@arm.com>
> Sent: 07 August 2026 14:05
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Alex Coplan <Alex.Coplan@arm.com>; Alice
> Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com;
> ktkachov@nvidia.com; Richard Earnshaw <Richard.Earnshaw@arm.com>;
> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> Subject: Re: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> On 07/08/2026 13:16, Tamar Christina wrote:
> > Hi Alfie,
> >
> > The 08/07/2026 11:31, Alfie Richards wrote:
> >> Hi Tamar,
> >> Sorry replying from phone so poor formatting.
> >>
> >>> I think the idea is good, but I don't quite like that now every first faulting
> load shape
> >>> needs Its own new update ffr and that the same UNSPEC has different
> amount of arguments.
> >>
> >> I do agree this is the part of the patch I was most uncomfortable with.
> >>
> >>> And because these are pseudo registers and we only have one of them the
> REGNOs are the same
> >>> and so it assumes they are the same. The patch fixes it by adding the load
> arguments to the
> >>> UNSPEC_UPDATE_FFR to make them unique to the load.
> >>
> >> Essentially yes but it’s also because the whole FFR expression tree matches
> going back to the setffr instructions (I believe)
> >>
> >>> That should give us a unique aarch64_update_ffr_for_load and also like
> your patch still
> >>> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> easier to backport.
> >>
> >> Yeah I like that idea, and I think it should work. I did think about something
> similar but my only hesitation was around the situation where you do two
> identical loads. I like in that situation with my patch that the duplicate load and
> rdffr gets removed whereas with the solution you propose we would remove
> the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big
> issue as it’s a pretty nonsense case but that’s why I went the way I did.
> >>
> >
> > I'm not sure I understand which scenario this is.
> >
> > Applying your patch:
> >
> > #include <arm_sve.h>
> >
> > void
> > same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> > unsigned char *o2, unsigned long *r1,
> > unsigned long *r2)
> > {
> > svbool_t pt = svptrue_b8 ();
> >
> > svsetffr ();
> >
> > svuint8_t a = svldff1_u8 (pt, p);
> > unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >
> > svuint8_t b = svldff1_u8 (pt, p);
> > unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >
> > svst1_u8 (pt, o1, a);
> > svst1_u8 (pt, o2, b);
> > *r1 = n1;
> > *r2 = n2;
> > }
>
> Ah so it would need to be the same situation as the original case with
> the double setffr, which is important to make the "expression" trees
> identical. I guess this make this opimiation so restrictive to the point
> of being even more negligable.
>
> So:
>
> #include <arm_sve.h>
>
> void
> same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> unsigned char *o2, unsigned long*r1,
> unsigned long *r2)
> {
> svbool_t pt = svptrue_b8 ();
>
> svsetffr ();
>
> svuint8_t a = svldff1_u8 (pt, p);
> unsigned long n1 = svcntp_b8 (pt, svrdffr ());
>
> svsetffr ();
>
> svuint8_t b = svldff1_u8 (pt, p);
> unsigned long n2 = svcntp_b8 (pt, svrdffr ());
>
> svst1_u8 (pt, o1, a);
> svst1_u8 (pt, o2, b);
> *r1 = n1;
> *r2 = n2;
> }
>
> Gives:
>
> same_load_read_same_load_read:
> .LFB2:
> .cfi_startproc
> ptrue p7.b, all
> setffr
> ldff1b z31.b, p7/z, [x0]
> st1b z31.b, p7, [x1]
> rdffr p15.b
> cntp x5, p7, p15.b
> st1b z31.b, p7, [x2]
> str x5, [x3]
> str x5, [x4]
> ret
> .cfi_endproc
>
> But again, pretty niche case.
For that all you need is to use the value result of the load
as the unique token. So instead of const_int you have
(define_insn "aarch64_update_ffr_for_load"
[(set (reg:VNx16BI FFR_REGNUM)
(unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
(reg:VNx16BI FFR_REGNUM)
(match_operand 0 "register_operand" "w")]
UNSPEC_UPDATE_FFR))]
And then the emitters become
rtx res = e.use_contiguous_load_insn (icode);
emit_insn (gen_aarch64_update_ffr_for_load (res));
return res;
etc. When the optimizers prove the loads are the same the results
become the same pseudos or get an equivalent relationship set which
makes the UNSPEC_UPDATE_FFRs the same and so they can be CSEd.
Thanks,
Tamar
>
> Thanks,
> Alfie
> >
> > Or did I misunderstand the case you were talking about?
> >
> > Thanks,
> > Tamar
> >
> >> Thoughts? Happy to take your solution if you’d still rather.
> >>
> >> KR,
> >> Alfie
> >>
> >> ________________________________
> >> From: Tamar Christina <Tamar.Christina@arm.com>
> >> Sent: 07 August 2026 09:23
> >> To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org
> <gcc-patches@gcc.gnu.org>
> >> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>;
> ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>;
> iains@gcc.gnu.org <iains@gcc.gnu.org>
> >> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >>
> >>> -----Original Message-----
> >>> From: Alfie Richards <alfie.richards@arm.com>
> >>> Sent: 05 August 2026 16:42
> >>> To: gcc-patches@gcc.gnu.org
> >>> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> >>> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> >>> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard
> Earnshaw
> >>> <Richard.Earnshaw@arm.com>; Tamar Christina
> <Tamar.Christina@arm.com>;
> >>> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> >>> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >>>
> >>> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> >>> the FFR loads to the associated FFR updates.
> >>>
> >>> This isn't quite perfectly true, as the hardware presumably could set
> >>> return different FFR values from the exact same load happening twice, but
> that
> >>> is a nonsense situation where the optimisation is valid.
> >>>
> >>
> >> Hi Alfie,
> >>
> >> I think the idea is good, but I don't quite like that now every first faulting
> load shape
> >> needs Its own new update ffr and that the same UNSPEC has different
> amount of arguments.
> >>
> >> If I’m not mistaken the problem is that CSE lib essentially sees
> >>
> >> (set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)]
> >> UNSPEC_UPDATE_FFR))
> >> (set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)]
> >> UNSPEC_UPDATE_FFR))
> >>
> >> And because these are pseudo registers and we only have one of them the
> REGNOs are the same
> >> and so it assumes they are the same. The patch fixes it by adding the load
> arguments to the
> >> UNSPEC_UPDATE_FFR to make them unique to the load.
> >>
> >> But I think we can fix this simpler by just adding a unique token to the
> aarch64_update_ffr_for_load.
> >>
> >> Like
> >>
> >> (define_insn "aarch64_update_ffr_for_load"
> >> [(set (reg:VNx16BI FFR_REGNUM)
> >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >> (reg:VNx16BI FFR_REGNUM)
> >> (match_operand 0 "const_int_operand" "n")]
> >> UNSPEC_UPDATE_FFR))]
> >>
> >> Then change
> >>
> >> emit_insn (gen_aarch64_update_ffr_for_load ());
> >>
> >> into
> >>
> >> emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
> >>
> >> the exact value of which doesn't really matter as long as it's unique (maybe
> make a helper).
> >>
> >> That should give us a unique aarch64_update_ffr_for_load and also like
> your patch still
> >> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> easier to backport.
> >>
> >> Thanks,
> >> Tamar
> >>
> >>> PR 126629
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * config/aarch64/aarch64-sve-builtins-base.cc:
> >>> (svldff1_gather_impl::expand): Remove
> >>> gen_aarch64_update_ffr_for_load.
> >>> (svldff1_svldff1_gather_extend::expand): Ditto.
> >>> (svldxf1_impl::expand): Ditto.
> >>> (svldxf1_extend_impl::expand): Ditto.
> >>> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> >>> Remove.
> >>> (*aarch64_update_ffr_for_continuous_load): New.
> >>> (*aarch64_update_ffr_for_gather_load): New.
> >>> (@aarch64_ld<fn>f1<mode>): New.
> >>> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> >>> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> >>> PARTIAL_I:mode>): New.
> >>> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> >>> ARTIAL_I:mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather<mode>): New.
> >>> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> >>> <VNx4_NARROW:mode>): New.
> >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> >>> VNx4_NARROW:mode>): Change to not be expanded directly.
> >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> >>> <VNx2_NARROW:mode>): New.
> >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> >>> VNx2_NARROW:mode>): Change to not be expanded directly.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>> * gcc.target/aarch64/sve/pr126629.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> >>> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
> >>>
> >>> -- >8 --
> >>>
> >>> Bootstrapped and regression tested on aarch64.
> >>>
> >>> Okay for master and backport?
> >>>
> >>> Thanks,
> >>> Alfie
> >>>
> >>> ---
> >>> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> >>> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> >>> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> >>> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> >>> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> >>> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> >>> 6 files changed, 279 insertions(+), 25 deletions(-)
> >>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> create mode 100644
> >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>>
> >>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> index 7f047bb6468..186f223ac63 100644
> >>> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> >>> @@ -2001,7 +2001,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> e.prepare_gather_address_operands (1);
> >>> /* Put the predicate last, since ldff1_gather uses the same operand
> >>> @@ -2023,7 +2022,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> e.prepare_gather_address_operands (1);
> >>> /* Put the predicate last, since ldff1_gather uses the same operand
> >>> @@ -2075,7 +2073,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> machine_mode mode = e.vector_mode (0);
> >>> return e.use_contiguous_load_insn (code_for_aarch64_ldf1
> (m_unspec,
> >>> mode));
> >>> @@ -2103,7 +2100,6 @@ public:
> >>> {
> >>> /* See the block comment in aarch64-sve.md for details about the
> >>> FFR handling. */
> >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> >>>
> >>> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code
> (),
> >>> e.vector_mode (0),
> >>> diff --git a/gcc/config/aarch64/aarch64-sve.md
> >>> b/gcc/config/aarch64/aarch64-sve.md
> >>> index 105b34eb8fa..1e131cc404a 100644
> >>> --- a/gcc/config/aarch64/aarch64-sve.md
> >>> +++ b/gcc/config/aarch64/aarch64-sve.md
> >>> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> >>> ;; so that the FFR value is live on entry to the region and so that the FFR
> >>> ;; value visibly changes within the region. This is used (possibly multiple
> >>> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> >>> -(define_insn "aarch64_update_ffr_for_load"
> >>> +(define_insn "*aarch64_update_ffr_for_continuous_load"
> >>> [(set (reg:VNx16BI FFR_REGNUM)
> >>> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> >>> - (reg:VNx16BI FFR_REGNUM)]
> >>> UNSPEC_UPDATE_FFR))]
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_operand 0 "" "X")
> >>> + (match_operand 1 "" "X")
> >>> + ] UNSPEC_UPDATE_FFR))]
> >>> + "TARGET_SVE"
> >>> + ""
> >>> + [(set_attr "type" "no_insn")]
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_update_ffr_for_gather_load"
> >>> + [(set (reg:VNx16BI FFR_REGNUM)
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_operand 0 "" "X")
> >>> + (match_operand 1 "" "X")
> >>> + (match_operand 2 "" "X")
> >>> + (match_operand 3 "" "X")
> >>> + (match_operand 4 "" "X")
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )]
> >>> "TARGET_SVE"
> >>> ""
> >>> [(set_attr "type" "no_insn")]
> >>> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> >>> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> >>> ;; -------------------------------------------------------------------------
> >>>
> >>> ;; Contiguous non-extending first-faulting or non-faulting loads.
> >>> -(define_insn "@aarch64_ld<fn>f1<mode>"
> >>> +(define_expand "@aarch64_ld<fn>f1<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 2)
> >>> + (match_dup 1)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL 0 "register_operand")
> >>> + (unspec:SVE_FULL
> >>> + [(match_operand:<VPRED> 2 "register_operand")
> >>> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + SVE_LDFF1_LDNF1))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ld<fn>f1<mode>"
> >>> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> >>> (unspec:SVE_FULL
> >>> [(match_operand:<VPRED> 2 "register_operand" "Upl")
> >>> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> >>> ;; -------------------------------------------------------------------------
> >>>
> >>> ;; Predicated first-faulting or non-faulting load and extend.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> >>> _I:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> >>> _I:mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 2)
> >>> + (match_dup 1)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_HSDI 0 "register_operand")
> >>> + (unspec:SVE_HSDI
> >>> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> >>> + (ANY_EXTEND:SVE_HSDI
> >>> + (unspec:SVE_PARTIAL_I
> >>> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> >>> + (match_operand:SVE_PARTIAL_I 1
> >>> "aarch64_sve_ld<fn>f1_operand")
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + SVE_LDFF1_LDNF1))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE
> >>> + && TARGET_NON_STREAMING
> >>> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) ==
> 0"
> >>> + {
> >>> + if (!CONSTANT_P (operands[3]))
> >>> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> >>> + }
> >>> +)
> >>> +
> >>> +;; Predicated first-faulting or non-faulting load and extend.
> >>> +(define_insn
> >>>
> "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> >>> I:mode>"
> >>> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> >>> (unspec:SVE_HSDI
> >>> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> >>> "UplDnm")
> >>> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> >>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> >>> && TARGET_NON_STREAMING
> >>> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> >>>
> >>>
> "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> >>> %2/z, %1"
> >>> - "&& !CONSTANT_P (operands[3])"
> >>> - {
> >>> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_load_1reg")]
> >>> )
> >>>
> >>> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> >>> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
> >>>
> >>> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> >>> ;; 3 is true for unsigned extension and false for signed extension.
> >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual
> load
> >>> + (unspec:SVE_FULL_S
> >>> + [(match_operand:VNx4BI 5 "register_operand")
> >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> >>> + (match_operand:VNx4SI 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> >>> [(set (match_operand:SVE_FULL_S 0 "register_operand")
> >>> (unspec:SVE_FULL_S
> >>> [(match_operand:VNx4BI 5 "register_operand")
> >>> @@ -1938,7 +2029,33 @@ (define_insn
> "@aarch64_ldff1_gather<mode>"
> >>>
> >>> ;; Predicated first-faulting gather loads for 64-bit elements. The value
> >>> ;; of operand 3 doesn't matter in this case.
> >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:SVE_FULL_D 0 "register_operand")
> >>> + (unspec:SVE_FULL_D
> >>> + [(match_operand:VNx2BI 5 "register_operand")
> >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> >>> + (match_operand:VNx2DI 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {}
> >>> +)
> >>> +
> >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> >>> [(set (match_operand:SVE_FULL_D 0 "register_operand")
> >>> (unspec:SVE_FULL_D
> >>> [(match_operand:VNx2BI 5 "register_operand")
> >>> @@ -2032,7 +2149,40 @@ (define_insn
> >>> "*aarch64_ldff1_gather<mode>_uxtw"
> >>>
> >>> ;; Predicated extending first-faulting gather loads for 32-bit elements.
> >>> ;; Operand 3 is true for unsigned extension and false for signed extension.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> >>> NARROW:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> >>> NARROW:mode>"
> >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> >>> + (unspec:VNx16BI
> >>> + [(reg:VNx16BI FFRT_REGNUM)
> >>> + (reg:VNx16BI FFR_REGNUM)
> >>> + (match_dup 5)
> >>> + (match_dup 1)
> >>> + (match_dup 2)
> >>> + (match_dup 3)
> >>> + (match_dup 4)
> >>> + ] UNSPEC_UPDATE_FFR)
> >>> + )
> >>> + (set (match_operand:VNx4_WIDE 0 "register_operand")
> >>> + (unspec:VNx4_WIDE
> >>> + [(match_operand:VNx4BI 6 "general_operand")
> >>> + (ANY_EXTEND:VNx4_WIDE
> >>> + (unspec:VNx4_NARROW
> >>> + [(match_operand:VNx4BI 5 "register_operand")
> >>> + (match_operand:DI 1
> >>> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> >>> + (match_operand:VNx4_WIDE 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4
> >>> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {
> >>> + if (!CONSTANT_P (operands[6]))
> >>> + operands[6] = CONSTM1_RTX (VNx4BImode);
> >>> + }
> >>> +)
> >>> +
> >>> +(define_insn
> >>>
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> >>> ARROW:mode>"
> >>> [(set (match_operand:VNx4_WIDE 0 "register_operand")
> >>> (unspec:VNx4_WIDE
> >>> [(match_operand:VNx4BI 6 "general_operand")
> >>> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> >>> [&w, rk, w, Ui1, i, Upl, UplDnm]
> >>> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s,
> uxtw
> >>> %p4]
> >>> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> >>> }
> >>> - "&& !CONSTANT_P (operands[6])"
> >>> - {
> >>> - operands[6] = CONSTM1_RTX (VNx4BImode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_gatherload_32")]
> >>> )
> >>>
> >>> ;; Predicated extending first-faulting gather loads for 64-bit elements.
> >>> ;; The value of operand 3 doesn't matter in this case.
> >>> -(define_insn_and_rewrite
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> >>> NARROW:mode>"
> >>> +(define_expand
> >>>
> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> >>> NARROW:mode>"
> >>> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> >>> + (unspec:VNx2_WIDE
> >>> + [(match_operand:VNx2BI 6 "general_operand")
> >>> + (ANY_EXTEND:VNx2_WIDE
> >>> + (unspec:VNx2_NARROW
> >>> + [(match_operand:VNx2BI 5 "register_operand")
> >>> + (match_operand:DI 1
> >>> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> >>> + (match_operand:VNx2_WIDE 2 "register_operand")
> >>> + (match_operand:DI 3 "const_int_operand")
> >>> + (match_operand:DI 4
> >>> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> >>> + (mem:BLK (scratch))
> >>> + (reg:VNx16BI FFRT_REGNUM)]
> >>> + UNSPEC_LDFF1_GATHER))]
> >>> + UNSPEC_PRED_X))]
> >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> >>> + {
> >>> + if (!CONSTANT_P (operands[6]))
> >>> + operands[6] = CONSTM1_RTX (VNx2BImode);
> >>> + }
> >>> +)
> >>> +
> >>> +(define_insn
> >>>
> "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> >>> ARROW:mode>"
> >>> [(set (match_operand:VNx2_WIDE 0 "register_operand")
> >>> (unspec:VNx2_WIDE
> >>> [(match_operand:VNx2BI 6 "general_operand")
> >>> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> >>> [&w, rk, w, i, i, Upl, UplDnm]
> >>> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d,
> lsl
> >>> %p4]
> >>> [?w, rk, w, i, i, Upl, UplDnm] ^
> >>> }
> >>> - "&& !CONSTANT_P (operands[6])"
> >>> - {
> >>> - operands[6] = CONSTM1_RTX (VNx2BImode);
> >>> - }
> >>> [(set_attr "sve_type" "sve_gatherload_64")]
> >>> )
> >>>
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000<tel:00000000000>>..64cb3ece330
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> >>> @@ -0,0 +1,23 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint8_t a = svldff1_u8 (pt, p);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint8_t b = svldff1_u8 (pt, q);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u8 (pt, o1, a);
> >>> + svst1_u8 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000<tel:00000000000>>..909a9f1e651
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> >>> @@ -0,0 +1,23 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1ub_u32 (pt, p);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1ub_u32 (pt, q);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000<tel:00000000000>>..2348082fde5
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> >>> @@ -0,0 +1,24 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> >>> + svuint32_t offset1, svuint32_t offset2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> new file mode 100644
> >>> index 00000000000<tel:00000000000<tel:00000000000>>..bb9257fe382
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> >>> @@ -0,0 +1,24 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O3" } */
> >>> +
> >>> +#include <arm_sve.h>
> >>> +
> >>> + void
> >>> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> >>> + long int offset1, long int offset2)
> >>> +{
> >>> + svbool_t pt = svptrue_b8 ();
> >>> + svsetffr ();
> >>> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >>> + svsetffr ();
> >>> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >>> + svst1_u32 (pt, o1, a);
> >>> + svst1_u32 (pt, o2, b);
> >>> + *r1 = n1;
> >>> + *r2 = n2;
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> >>> --
> >>> 2.34.1
> >>
> >
The 08/10/2026 10:19, Alfie Richards wrote:
> Hi Tamar,
>
> (Sorry on my phone again)
>
> > For that all you need is to use the value result of the load
> > as the unique token. So instead of const_int you have
>
> > (define_insn "aarch64_update_ffr_for_load"
> > [(set (reg:VNx16BI FFR_REGNUM)
> > (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > (reg:VNx16BI FFR_REGNUM)
> > (match_operand 0 "register_operand" "w")]
> > UNSPEC_UPDATE_FFR))]
>
> I did consider this too but my concern then is that you force the load values live even if the results aren’t used. (Provided the ffr stays live which it often does) So we may lose DCE on the load.
>
> Sorry to be so picky about this, I’m aware it doesn’t really matter that much.
The only reason the loads would stay live is because the FFR group as a whole is live,
which means you have another load in same group which the result is needed.
But then you can't CSE the unused load because even though it's result isn't used it
influences the FFR.
So I don't think the situation you described above is valid to remove the load as FFRs
can only be CSE'd as a group, or not at all.
Thanks,
Tamar
>
> I think in balance the max_uid solution may be best? Happy to re-spin with that when I’m back.
>
> Thanks,
> Alfie
>
>
> ________________________________
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: 10 August 2026 08:10
> To: Alfie Richards <Alfie.Richards@arm.com>
> Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; Alex Coplan <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>; ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org <iains@gcc.gnu.org>
> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
>
> Hi Alfie,
>
> > -----Original Message-----
> > From: Alfie Richards <Alfie.Richards@arm.com>
> > Sent: 07 August 2026 14:05
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; Alex Coplan <Alex.Coplan@arm.com>; Alice
> > Carlotti <Alice.Carlotti@arm.com>; andrew.pinski@oss.qualcomm.com;
> > ktkachov@nvidia.com; Richard Earnshaw <Richard.Earnshaw@arm.com>;
> > Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> > Subject: Re: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> >
> > On 07/08/2026 13:16, Tamar Christina wrote:
> > > Hi Alfie,
> > >
> > > The 08/07/2026 11:31, Alfie Richards wrote:
> > >> Hi Tamar,
> > >> Sorry replying from phone so poor formatting.
> > >>
> > >>> I think the idea is good, but I don't quite like that now every first faulting
> > load shape
> > >>> needs Its own new update ffr and that the same UNSPEC has different
> > amount of arguments.
> > >>
> > >> I do agree this is the part of the patch I was most uncomfortable with.
> > >>
> > >>> And because these are pseudo registers and we only have one of them the
> > REGNOs are the same
> > >>> and so it assumes they are the same. The patch fixes it by adding the load
> > arguments to the
> > >>> UNSPEC_UPDATE_FFR to make them unique to the load.
> > >>
> > >> Essentially yes but it’s also because the whole FFR expression tree matches
> > going back to the setffr instructions (I believe)
> > >>
> > >>> That should give us a unique aarch64_update_ffr_for_load and also like
> > your patch still
> > >>> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> > easier to backport.
> > >>
> > >> Yeah I like that idea, and I think it should work. I did think about something
> > similar but my only hesitation was around the situation where you do two
> > identical loads. I like in that situation with my patch that the duplicate load and
> > rdffr gets removed whereas with the solution you propose we would remove
> > the ldffr but keep the duplicate rdffr (and hidden non-instructions). Not a big
> > issue as it’s a pretty nonsense case but that’s why I went the way I did.
> > >>
> > >
> > > I'm not sure I understand which scenario this is.
> > >
> > > Applying your patch:
> > >
> > > #include <arm_sve.h>
> > >
> > > void
> > > same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> > > unsigned char *o2, unsigned long *r1,
> > > unsigned long *r2)
> > > {
> > > svbool_t pt = svptrue_b8 ();
> > >
> > > svsetffr ();
> > >
> > > svuint8_t a = svldff1_u8 (pt, p);
> > > unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > >
> > > svuint8_t b = svldff1_u8 (pt, p);
> > > unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > >
> > > svst1_u8 (pt, o1, a);
> > > svst1_u8 (pt, o2, b);
> > > *r1 = n1;
> > > *r2 = n2;
> > > }
> >
> > Ah so it would need to be the same situation as the original case with
> > the double setffr, which is important to make the "expression" trees
> > identical. I guess this make this opimiation so restrictive to the point
> > of being even more negligable.
> >
> > So:
> >
> > #include <arm_sve.h>
> >
> > void
> > same_load_read_same_load_read (unsigned char *p, unsigned char *o1,
> > unsigned char *o2, unsigned long*r1,
> > unsigned long *r2)
> > {
> > svbool_t pt = svptrue_b8 ();
> >
> > svsetffr ();
> >
> > svuint8_t a = svldff1_u8 (pt, p);
> > unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> >
> > svsetffr ();
> >
> > svuint8_t b = svldff1_u8 (pt, p);
> > unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> >
> > svst1_u8 (pt, o1, a);
> > svst1_u8 (pt, o2, b);
> > *r1 = n1;
> > *r2 = n2;
> > }
> >
> > Gives:
> >
> > same_load_read_same_load_read:
> > .LFB2:
> > .cfi_startproc
> > ptrue p7.b, all
> > setffr
> > ldff1b z31.b, p7/z, [x0]
> > st1b z31.b, p7, [x1]
> > rdffr p15.b
> > cntp x5, p7, p15.b
> > st1b z31.b, p7, [x2]
> > str x5, [x3]
> > str x5, [x4]
> > ret
> > .cfi_endproc
> >
> > But again, pretty niche case.
>
> For that all you need is to use the value result of the load
> as the unique token. So instead of const_int you have
>
> (define_insn "aarch64_update_ffr_for_load"
> [(set (reg:VNx16BI FFR_REGNUM)
> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> (reg:VNx16BI FFR_REGNUM)
> (match_operand 0 "register_operand" "w")]
> UNSPEC_UPDATE_FFR))]
>
> And then the emitters become
>
> rtx res = e.use_contiguous_load_insn (icode);
> emit_insn (gen_aarch64_update_ffr_for_load (res));
> return res;
>
> etc. When the optimizers prove the loads are the same the results
> become the same pseudos or get an equivalent relationship set which
> makes the UNSPEC_UPDATE_FFRs the same and so they can be CSEd.
>
> Thanks,
> Tamar
>
> >
> > Thanks,
> > Alfie
> > >
> > > Or did I misunderstand the case you were talking about?
> > >
> > > Thanks,
> > > Tamar
> > >
> > >> Thoughts? Happy to take your solution if you’d still rather.
> > >>
> > >> KR,
> > >> Alfie
> > >>
> > >> ________________________________
> > >> From: Tamar Christina <Tamar.Christina@arm.com>
> > >> Sent: 07 August 2026 09:23
> > >> To: Alfie Richards <Alfie.Richards@arm.com>; gcc-patches@gcc.gnu.org
> > <gcc-patches@gcc.gnu.org>
> > >> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> > <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> > andrew.pinski@oss.qualcomm.com <andrew.pinski@oss.qualcomm.com>;
> > ktkachov@nvidia.com <ktkachov@nvidia.com>; Richard Earnshaw
> > <Richard.Earnshaw@arm.com>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>;
> > iains@gcc.gnu.org <iains@gcc.gnu.org>
> > >> Subject: RE: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> > >>
> > >>> -----Original Message-----
> > >>> From: Alfie Richards <alfie.richards@arm.com>
> > >>> Sent: 05 August 2026 16:42
> > >>> To: gcc-patches@gcc.gnu.org
> > >>> Cc: Alfie Richards <Alfie.Richards@arm.com>; Alex Coplan
> > >>> <Alex.Coplan@arm.com>; Alice Carlotti <Alice.Carlotti@arm.com>;
> > >>> andrew.pinski@oss.qualcomm.com; ktkachov@nvidia.com; Richard
> > Earnshaw
> > >>> <Richard.Earnshaw@arm.com>; Tamar Christina
> > <Tamar.Christina@arm.com>;
> > >>> Wilco Dijkstra <Wilco.Dijkstra@arm.com>; iains@gcc.gnu.org
> > >>> Subject: [PATCH] aarch64: Fix invalid CSE for RDRFFS [PR 126629]
> > >>>
> > >>> Fixes the invalid CSE'ing of the FFR reads by adding the arguments from
> > >>> the FFR loads to the associated FFR updates.
> > >>>
> > >>> This isn't quite perfectly true, as the hardware presumably could set
> > >>> return different FFR values from the exact same load happening twice, but
> > that
> > >>> is a nonsense situation where the optimisation is valid.
> > >>>
> > >>
> > >> Hi Alfie,
> > >>
> > >> I think the idea is good, but I don't quite like that now every first faulting
> > load shape
> > >> needs Its own new update ffr and that the same UNSPEC has different
> > amount of arguments.
> > >>
> > >> If I’m not mistaken the problem is that CSE lib essentially sees
> > >>
> > >> (set (reg:VNx16BI FFR_REGNUM)
> > >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > >> (reg:VNx16BI FFR_REGNUM)]
> > >> UNSPEC_UPDATE_FFR))
> > >> (set (reg:VNx16BI FFR_REGNUM)
> > >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > >> (reg:VNx16BI FFR_REGNUM)]
> > >> UNSPEC_UPDATE_FFR))
> > >>
> > >> And because these are pseudo registers and we only have one of them the
> > REGNOs are the same
> > >> and so it assumes they are the same. The patch fixes it by adding the load
> > arguments to the
> > >> UNSPEC_UPDATE_FFR to make them unique to the load.
> > >>
> > >> But I think we can fix this simpler by just adding a unique token to the
> > aarch64_update_ffr_for_load.
> > >>
> > >> Like
> > >>
> > >> (define_insn "aarch64_update_ffr_for_load"
> > >> [(set (reg:VNx16BI FFR_REGNUM)
> > >> (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > >> (reg:VNx16BI FFR_REGNUM)
> > >> (match_operand 0 "const_int_operand" "n")]
> > >> UNSPEC_UPDATE_FFR))]
> > >>
> > >> Then change
> > >>
> > >> emit_insn (gen_aarch64_update_ffr_for_load ());
> > >>
> > >> into
> > >>
> > >> emit_insn (gen_aarch64_update_ffr_for_load (GEN_INT (get_max_uid ())));
> > >>
> > >> the exact value of which doesn't really matter as long as it's unique (maybe
> > make a helper).
> > >>
> > >> That should give us a unique aarch64_update_ffr_for_load and also like
> > your patch still
> > >> allow removals of redundant setffrs and rdffrs but be a bit simpler and
> > easier to backport.
> > >>
> > >> Thanks,
> > >> Tamar
> > >>
> > >>> PR 126629
> > >>>
> > >>> gcc/ChangeLog:
> > >>>
> > >>> * config/aarch64/aarch64-sve-builtins-base.cc:
> > >>> (svldff1_gather_impl::expand): Remove
> > >>> gen_aarch64_update_ffr_for_load.
> > >>> (svldff1_svldff1_gather_extend::expand): Ditto.
> > >>> (svldxf1_impl::expand): Ditto.
> > >>> (svldxf1_extend_impl::expand): Ditto.
> > >>> * config/aarch64/aarch64-sve.md (aarch64_update_ffr_for_load):
> > >>> Remove.
> > >>> (*aarch64_update_ffr_for_continuous_load): New.
> > >>> (*aarch64_update_ffr_for_gather_load): New.
> > >>> (@aarch64_ld<fn>f1<mode>): New.
> > >>> (*aarch64_ld<fn>f1<mode>): Change to not be expanded directly.
> > >>> (@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_
> > >>> PARTIAL_I:mode>): New.
> > >>> (*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_P
> > >>> ARTIAL_I:mode>): Change to not be expanded directly.
> > >>> (@aarch64_ldff1_gather<mode>): New.
> > >>> (*aarch64_ldff1_gather<mode>): Change to not be expanded directly.
> > >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode>
> > >>> <VNx4_NARROW:mode>): New.
> > >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><
> > >>> VNx4_NARROW:mode>): Change to not be expanded directly.
> > >>> (@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode>
> > >>> <VNx2_NARROW:mode>): New.
> > >>> (*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><
> > >>> VNx2_NARROW:mode>): Change to not be expanded directly.
> > >>>
> > >>> gcc/testsuite/ChangeLog:
> > >>>
> > >>> * gcc.target/aarch64/sve/pr126629.c: New test.
> > >>> * gcc.target/aarch64/sve/pr126629_extend.c: New test.
> > >>> * gcc.target/aarch64/sve/pr126629_gather.c: New test.
> > >>> * gcc.target/aarch64/sve/pr126629_gather2.c: New test.
> > >>>
> > >>> -- >8 --
> > >>>
> > >>> Bootstrapped and regression tested on aarch64.
> > >>>
> > >>> Okay for master and backport?
> > >>>
> > >>> Thanks,
> > >>> Alfie
> > >>>
> > >>> ---
> > >>> .../aarch64/aarch64-sve-builtins-base.cc | 4 -
> > >>> gcc/config/aarch64/aarch64-sve.md | 206 ++++++++++++++++--
> > >>> .../gcc.target/aarch64/sve/pr126629.c | 23 ++
> > >>> .../gcc.target/aarch64/sve/pr126629_extend.c | 23 ++
> > >>> .../gcc.target/aarch64/sve/pr126629_gather.c | 24 ++
> > >>> .../gcc.target/aarch64/sve/pr126629_gather2.c | 24 ++
> > >>> 6 files changed, 279 insertions(+), 25 deletions(-)
> > >>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > >>> create mode 100644
> > >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > >>> create mode 100644
> > >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > >>> create mode 100644
> > >>> gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > >>>
> > >>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > >>> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > >>> index 7f047bb6468..186f223ac63 100644
> > >>> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > >>> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > >>> @@ -2001,7 +2001,6 @@ public:
> > >>> {
> > >>> /* See the block comment in aarch64-sve.md for details about the
> > >>> FFR handling. */
> > >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> > >>>
> > >>> e.prepare_gather_address_operands (1);
> > >>> /* Put the predicate last, since ldff1_gather uses the same operand
> > >>> @@ -2023,7 +2022,6 @@ public:
> > >>> {
> > >>> /* See the block comment in aarch64-sve.md for details about the
> > >>> FFR handling. */
> > >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> > >>>
> > >>> e.prepare_gather_address_operands (1);
> > >>> /* Put the predicate last, since ldff1_gather uses the same operand
> > >>> @@ -2075,7 +2073,6 @@ public:
> > >>> {
> > >>> /* See the block comment in aarch64-sve.md for details about the
> > >>> FFR handling. */
> > >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> > >>>
> > >>> machine_mode mode = e.vector_mode (0);
> > >>> return e.use_contiguous_load_insn (code_for_aarch64_ldf1
> > (m_unspec,
> > >>> mode));
> > >>> @@ -2103,7 +2100,6 @@ public:
> > >>> {
> > >>> /* See the block comment in aarch64-sve.md for details about the
> > >>> FFR handling. */
> > >>> - emit_insn (gen_aarch64_update_ffr_for_load ());
> > >>>
> > >>> insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code
> > (),
> > >>> e.vector_mode (0),
> > >>> diff --git a/gcc/config/aarch64/aarch64-sve.md
> > >>> b/gcc/config/aarch64/aarch64-sve.md
> > >>> index 105b34eb8fa..1e131cc404a 100644
> > >>> --- a/gcc/config/aarch64/aarch64-sve.md
> > >>> +++ b/gcc/config/aarch64/aarch64-sve.md
> > >>> @@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
> > >>> ;; so that the FFR value is live on entry to the region and so that the FFR
> > >>> ;; value visibly changes within the region. This is used (possibly multiple
> > >>> ;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
> > >>> -(define_insn "aarch64_update_ffr_for_load"
> > >>> +(define_insn "*aarch64_update_ffr_for_continuous_load"
> > >>> [(set (reg:VNx16BI FFR_REGNUM)
> > >>> - (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
> > >>> - (reg:VNx16BI FFR_REGNUM)]
> > >>> UNSPEC_UPDATE_FFR))]
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_operand 0 "" "X")
> > >>> + (match_operand 1 "" "X")
> > >>> + ] UNSPEC_UPDATE_FFR))]
> > >>> + "TARGET_SVE"
> > >>> + ""
> > >>> + [(set_attr "type" "no_insn")]
> > >>> +)
> > >>> +
> > >>> +(define_insn "*aarch64_update_ffr_for_gather_load"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM)
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_operand 0 "" "X")
> > >>> + (match_operand 1 "" "X")
> > >>> + (match_operand 2 "" "X")
> > >>> + (match_operand 3 "" "X")
> > >>> + (match_operand 4 "" "X")
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )]
> > >>> "TARGET_SVE"
> > >>> ""
> > >>> [(set_attr "type" "no_insn")]
> > >>> @@ -1448,7 +1469,26 @@ (define_insn_and_rewrite
> > >>> "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
> > >>> ;; -------------------------------------------------------------------------
> > >>>
> > >>> ;; Contiguous non-extending first-faulting or non-faulting loads.
> > >>> -(define_insn "@aarch64_ld<fn>f1<mode>"
> > >>> +(define_expand "@aarch64_ld<fn>f1<mode>"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_dup 2)
> > >>> + (match_dup 1)
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )
> > >>> + (set (match_operand:SVE_FULL 0 "register_operand")
> > >>> + (unspec:SVE_FULL
> > >>> + [(match_operand:<VPRED> 2 "register_operand")
> > >>> + (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + SVE_LDFF1_LDNF1))]
> > >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> > >>> + {}
> > >>> +)
> > >>> +
> > >>> +(define_insn "*aarch64_ld<fn>f1<mode>"
> > >>> [(set (match_operand:SVE_FULL 0 "register_operand" "=w")
> > >>> (unspec:SVE_FULL
> > >>> [(match_operand:<VPRED> 2 "register_operand" "Upl")
> > >>> @@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
> > >>> ;; -------------------------------------------------------------------------
> > >>>
> > >>> ;; Predicated first-faulting or non-faulting load and extend.
> > >>> -(define_insn_and_rewrite
> > >>>
> > "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> > >>> _I:mode>"
> > >>> +(define_expand
> > >>>
> > "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL
> > >>> _I:mode>"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_dup 2)
> > >>> + (match_dup 1)
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )
> > >>> + (set (match_operand:SVE_HSDI 0 "register_operand")
> > >>> + (unspec:SVE_HSDI
> > >>> + [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
> > >>> + (ANY_EXTEND:SVE_HSDI
> > >>> + (unspec:SVE_PARTIAL_I
> > >>> + [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
> > >>> + (match_operand:SVE_PARTIAL_I 1
> > >>> "aarch64_sve_ld<fn>f1_operand")
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + SVE_LDFF1_LDNF1))]
> > >>> + UNSPEC_PRED_X))]
> > >>> + "TARGET_SVE
> > >>> + && TARGET_NON_STREAMING
> > >>> + && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) ==
> > 0"
> > >>> + {
> > >>> + if (!CONSTANT_P (operands[3]))
> > >>> + operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> > >>> + }
> > >>> +)
> > >>> +
> > >>> +;; Predicated first-faulting or non-faulting load and extend.
> > >>> +(define_insn
> > >>>
> > "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_
> > >>> I:mode>"
> > >>> [(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
> > >>> (unspec:SVE_HSDI
> > >>> [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand"
> > >>> "UplDnm")
> > >>> @@ -1494,10 +1563,6 @@ (define_insn_and_rewrite
> > >>> "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
> > >>> && TARGET_NON_STREAMING
> > >>> && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
> > >>>
> > >>>
> > "ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>,
> > >>> %2/z, %1"
> > >>> - "&& !CONSTANT_P (operands[3])"
> > >>> - {
> > >>> - operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
> > >>> - }
> > >>> [(set_attr "sve_type" "sve_load_1reg")]
> > >>> )
> > >>>
> > >>> @@ -1907,7 +1972,33 @@ (define_insn_and_rewrite
> > >>> "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
> > >>>
> > >>> ;; Predicated first-faulting gather loads for 32-bit elements. Operand
> > >>> ;; 3 is true for unsigned extension and false for signed extension.
> > >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> > >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_dup 5)
> > >>> + (match_dup 1)
> > >>> + (match_dup 2)
> > >>> + (match_dup 3)
> > >>> + (match_dup 4)
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )
> > >>> + (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual
> > load
> > >>> + (unspec:SVE_FULL_S
> > >>> + [(match_operand:VNx4BI 5 "register_operand")
> > >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_w")
> > >>> + (match_operand:VNx4SI 2 "register_operand")
> > >>> + (match_operand:DI 3 "const_int_operand")
> > >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_w")
> > >>> + (mem:BLK (scratch))
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + UNSPEC_LDFF1_GATHER))]
> > >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> > >>> + {}
> > >>> +)
> > >>> +
> > >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> > >>> [(set (match_operand:SVE_FULL_S 0 "register_operand")
> > >>> (unspec:SVE_FULL_S
> > >>> [(match_operand:VNx4BI 5 "register_operand")
> > >>> @@ -1938,7 +2029,33 @@ (define_insn
> > "@aarch64_ldff1_gather<mode>"
> > >>>
> > >>> ;; Predicated first-faulting gather loads for 64-bit elements. The value
> > >>> ;; of operand 3 doesn't matter in this case.
> > >>> -(define_insn "@aarch64_ldff1_gather<mode>"
> > >>> +(define_expand "@aarch64_ldff1_gather<mode>"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_dup 5)
> > >>> + (match_dup 1)
> > >>> + (match_dup 2)
> > >>> + (match_dup 3)
> > >>> + (match_dup 4)
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )
> > >>> + (set (match_operand:SVE_FULL_D 0 "register_operand")
> > >>> + (unspec:SVE_FULL_D
> > >>> + [(match_operand:VNx2BI 5 "register_operand")
> > >>> + (match_operand:DI 1 "aarch64_sve_gather_offset_d")
> > >>> + (match_operand:VNx2DI 2 "register_operand")
> > >>> + (match_operand:DI 3 "const_int_operand")
> > >>> + (match_operand:DI 4 "aarch64_gather_scale_operand_d")
> > >>> + (mem:BLK (scratch))
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + UNSPEC_LDFF1_GATHER))]
> > >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> > >>> + {}
> > >>> +)
> > >>> +
> > >>> +(define_insn "*aarch64_ldff1_gather<mode>"
> > >>> [(set (match_operand:SVE_FULL_D 0 "register_operand")
> > >>> (unspec:SVE_FULL_D
> > >>> [(match_operand:VNx2BI 5 "register_operand")
> > >>> @@ -2032,7 +2149,40 @@ (define_insn
> > >>> "*aarch64_ldff1_gather<mode>_uxtw"
> > >>>
> > >>> ;; Predicated extending first-faulting gather loads for 32-bit elements.
> > >>> ;; Operand 3 is true for unsigned extension and false for signed extension.
> > >>> -(define_insn_and_rewrite
> > >>>
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> > >>> NARROW:mode>"
> > >>> +(define_expand
> > >>>
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_
> > >>> NARROW:mode>"
> > >>> + [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
> > >>> + (unspec:VNx16BI
> > >>> + [(reg:VNx16BI FFRT_REGNUM)
> > >>> + (reg:VNx16BI FFR_REGNUM)
> > >>> + (match_dup 5)
> > >>> + (match_dup 1)
> > >>> + (match_dup 2)
> > >>> + (match_dup 3)
> > >>> + (match_dup 4)
> > >>> + ] UNSPEC_UPDATE_FFR)
> > >>> + )
> > >>> + (set (match_operand:VNx4_WIDE 0 "register_operand")
> > >>> + (unspec:VNx4_WIDE
> > >>> + [(match_operand:VNx4BI 6 "general_operand")
> > >>> + (ANY_EXTEND:VNx4_WIDE
> > >>> + (unspec:VNx4_NARROW
> > >>> + [(match_operand:VNx4BI 5 "register_operand")
> > >>> + (match_operand:DI 1
> > >>> "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
> > >>> + (match_operand:VNx4_WIDE 2 "register_operand")
> > >>> + (match_operand:DI 3 "const_int_operand")
> > >>> + (match_operand:DI 4
> > >>> "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
> > >>> + (mem:BLK (scratch))
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + UNSPEC_LDFF1_GATHER))]
> > >>> + UNSPEC_PRED_X))]
> > >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> > >>> + {
> > >>> + if (!CONSTANT_P (operands[6]))
> > >>> + operands[6] = CONSTM1_RTX (VNx4BImode);
> > >>> + }
> > >>> +)
> > >>> +
> > >>> +(define_insn
> > >>>
> > "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_N
> > >>> ARROW:mode>"
> > >>> [(set (match_operand:VNx4_WIDE 0 "register_operand")
> > >>> (unspec:VNx4_WIDE
> > >>> [(match_operand:VNx4BI 6 "general_operand")
> > >>> @@ -2062,16 +2212,34 @@ (define_insn_and_rewrite
> > >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
> > >>> [&w, rk, w, Ui1, i, Upl, UplDnm]
> > >>> ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s,
> > uxtw
> > >>> %p4]
> > >>> [?w, rk, 0, Ui1, i, Upl, UplDnm] ^
> > >>> }
> > >>> - "&& !CONSTANT_P (operands[6])"
> > >>> - {
> > >>> - operands[6] = CONSTM1_RTX (VNx4BImode);
> > >>> - }
> > >>> [(set_attr "sve_type" "sve_gatherload_32")]
> > >>> )
> > >>>
> > >>> ;; Predicated extending first-faulting gather loads for 64-bit elements.
> > >>> ;; The value of operand 3 doesn't matter in this case.
> > >>> -(define_insn_and_rewrite
> > >>>
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> > >>> NARROW:mode>"
> > >>> +(define_expand
> > >>>
> > "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_
> > >>> NARROW:mode>"
> > >>> + [(set (match_operand:VNx2_WIDE 0 "register_operand")
> > >>> + (unspec:VNx2_WIDE
> > >>> + [(match_operand:VNx2BI 6 "general_operand")
> > >>> + (ANY_EXTEND:VNx2_WIDE
> > >>> + (unspec:VNx2_NARROW
> > >>> + [(match_operand:VNx2BI 5 "register_operand")
> > >>> + (match_operand:DI 1
> > >>> "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
> > >>> + (match_operand:VNx2_WIDE 2 "register_operand")
> > >>> + (match_operand:DI 3 "const_int_operand")
> > >>> + (match_operand:DI 4
> > >>> "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
> > >>> + (mem:BLK (scratch))
> > >>> + (reg:VNx16BI FFRT_REGNUM)]
> > >>> + UNSPEC_LDFF1_GATHER))]
> > >>> + UNSPEC_PRED_X))]
> > >>> + "TARGET_SVE && TARGET_NON_STREAMING"
> > >>> + {
> > >>> + if (!CONSTANT_P (operands[6]))
> > >>> + operands[6] = CONSTM1_RTX (VNx2BImode);
> > >>> + }
> > >>> +)
> > >>> +
> > >>> +(define_insn
> > >>>
> > "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_N
> > >>> ARROW:mode>"
> > >>> [(set (match_operand:VNx2_WIDE 0 "register_operand")
> > >>> (unspec:VNx2_WIDE
> > >>> [(match_operand:VNx2BI 6 "general_operand")
> > >>> @@ -2097,10 +2265,6 @@ (define_insn_and_rewrite
> > >>> "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
> > >>> [&w, rk, w, i, i, Upl, UplDnm]
> > >>> ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d,
> > lsl
> > >>> %p4]
> > >>> [?w, rk, w, i, i, Upl, UplDnm] ^
> > >>> }
> > >>> - "&& !CONSTANT_P (operands[6])"
> > >>> - {
> > >>> - operands[6] = CONSTM1_RTX (VNx2BImode);
> > >>> - }
> > >>> [(set_attr "sve_type" "sve_gatherload_64")]
> > >>> )
> > >>>
> > >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > >>> new file mode 100644
> > >>> index 00000000000<tel:00000000000<tel:00000000000>>..64cb3ece330
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629.c
> > >>> @@ -0,0 +1,23 @@
> > >>> +/* { dg-do compile } */
> > >>> +/* { dg-options "-O3" } */
> > >>> +
> > >>> +#include <arm_sve.h>
> > >>> +
> > >>> + void
> > >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > >>> + unsigned long *r2, unsigned char *o1, unsigned char *o2)
> > >>> +{
> > >>> + svbool_t pt = svptrue_b8 ();
> > >>> + svsetffr ();
> > >>> + svuint8_t a = svldff1_u8 (pt, p);
> > >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > >>> + svsetffr ();
> > >>> + svuint8_t b = svldff1_u8 (pt, q);
> > >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > >>> + svst1_u8 (pt, o1, a);
> > >>> + svst1_u8 (pt, o2, b);
> > >>> + *r1 = n1;
> > >>> + *r2 = n2;
> > >>> +}
> > >>> +
> > >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > >>> new file mode 100644
> > >>> index 00000000000<tel:00000000000<tel:00000000000>>..909a9f1e651
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_extend.c
> > >>> @@ -0,0 +1,23 @@
> > >>> +/* { dg-do compile } */
> > >>> +/* { dg-options "-O3" } */
> > >>> +
> > >>> +#include <arm_sve.h>
> > >>> +
> > >>> + void
> > >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > >>> + unsigned long *r2, unsigned int *o1, unsigned int *o2)
> > >>> +{
> > >>> + svbool_t pt = svptrue_b8 ();
> > >>> + svsetffr ();
> > >>> + svuint32_t a = svldff1ub_u32 (pt, p);
> > >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > >>> + svsetffr ();
> > >>> + svuint32_t b = svldff1ub_u32 (pt, q);
> > >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > >>> + svst1_u32 (pt, o1, a);
> > >>> + svst1_u32 (pt, o2, b);
> > >>> + *r1 = n1;
> > >>> + *r2 = n2;
> > >>> +}
> > >>> +
> > >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > >>> new file mode 100644
> > >>> index 00000000000<tel:00000000000<tel:00000000000>>..2348082fde5
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather.c
> > >>> @@ -0,0 +1,24 @@
> > >>> +/* { dg-do compile } */
> > >>> +/* { dg-options "-O3" } */
> > >>> +
> > >>> +#include <arm_sve.h>
> > >>> +
> > >>> + void
> > >>> +two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
> > >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> > >>> + svuint32_t offset1, svuint32_t offset2)
> > >>> +{
> > >>> + svbool_t pt = svptrue_b8 ();
> > >>> + svsetffr ();
> > >>> + svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
> > >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > >>> + svsetffr ();
> > >>> + svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
> > >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > >>> + svst1_u32 (pt, o1, a);
> > >>> + svst1_u32 (pt, o2, b);
> > >>> + *r1 = n1;
> > >>> + *r2 = n2;
> > >>> +}
> > >>> +
> > >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > >>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > >>> b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > >>> new file mode 100644
> > >>> index 00000000000<tel:00000000000<tel:00000000000>>..bb9257fe382
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126629_gather2.c
> > >>> @@ -0,0 +1,24 @@
> > >>> +/* { dg-do compile } */
> > >>> +/* { dg-options "-O3" } */
> > >>> +
> > >>> +#include <arm_sve.h>
> > >>> +
> > >>> + void
> > >>> +two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
> > >>> + unsigned long *r2, unsigned *o1, unsigned *o2,
> > >>> + long int offset1, long int offset2)
> > >>> +{
> > >>> + svbool_t pt = svptrue_b8 ();
> > >>> + svsetffr ();
> > >>> + svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
> > >>> + unsigned long n1 = svcntp_b8 (pt, svrdffr ());
> > >>> + svsetffr ();
> > >>> + svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
> > >>> + unsigned long n2 = svcntp_b8 (pt, svrdffr ());
> > >>> + svst1_u32 (pt, o1, a);
> > >>> + svst1_u32 (pt, o2, b);
> > >>> + *r1 = n1;
> > >>> + *r2 = n2;
> > >>> +}
> > >>> +
> > >>> +/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
> > >>> --
> > >>> 2.34.1
> > >>
> > >
>
--
@@ -2001,7 +2001,6 @@ public:
{
/* See the block comment in aarch64-sve.md for details about the
FFR handling. */
- emit_insn (gen_aarch64_update_ffr_for_load ());
e.prepare_gather_address_operands (1);
/* Put the predicate last, since ldff1_gather uses the same operand
@@ -2023,7 +2022,6 @@ public:
{
/* See the block comment in aarch64-sve.md for details about the
FFR handling. */
- emit_insn (gen_aarch64_update_ffr_for_load ());
e.prepare_gather_address_operands (1);
/* Put the predicate last, since ldff1_gather uses the same operand
@@ -2075,7 +2073,6 @@ public:
{
/* See the block comment in aarch64-sve.md for details about the
FFR handling. */
- emit_insn (gen_aarch64_update_ffr_for_load ());
machine_mode mode = e.vector_mode (0);
return e.use_contiguous_load_insn (code_for_aarch64_ldf1 (m_unspec, mode));
@@ -2103,7 +2100,6 @@ public:
{
/* See the block comment in aarch64-sve.md for details about the
FFR handling. */
- emit_insn (gen_aarch64_update_ffr_for_load ());
insn_code icode = code_for_aarch64_ldf1 (m_unspec, extend_rtx_code (),
e.vector_mode (0),
@@ -1150,10 +1150,31 @@ (define_insn "aarch64_wrffr"
;; so that the FFR value is live on entry to the region and so that the FFR
;; value visibly changes within the region. This is used (possibly multiple
;; times) in an FFRT region that includes LDFF1 or LDNF1 instructions.
-(define_insn "aarch64_update_ffr_for_load"
+(define_insn "*aarch64_update_ffr_for_continuous_load"
[(set (reg:VNx16BI FFR_REGNUM)
- (unspec:VNx16BI [(reg:VNx16BI FFRT_REGNUM)
- (reg:VNx16BI FFR_REGNUM)] UNSPEC_UPDATE_FFR))]
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_operand 0 "" "X")
+ (match_operand 1 "" "X")
+ ] UNSPEC_UPDATE_FFR))]
+ "TARGET_SVE"
+ ""
+ [(set_attr "type" "no_insn")]
+)
+
+(define_insn "*aarch64_update_ffr_for_gather_load"
+ [(set (reg:VNx16BI FFR_REGNUM)
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_operand 0 "" "X")
+ (match_operand 1 "" "X")
+ (match_operand 2 "" "X")
+ (match_operand 3 "" "X")
+ (match_operand 4 "" "X")
+ ] UNSPEC_UPDATE_FFR)
+ )]
"TARGET_SVE"
""
[(set_attr "type" "no_insn")]
@@ -1448,7 +1469,26 @@ (define_insn_and_rewrite "*aarch64_load_<ANY_EXTEND:optab>_mov<SVE_HSDI:mode><SV
;; -------------------------------------------------------------------------
;; Contiguous non-extending first-faulting or non-faulting loads.
-(define_insn "@aarch64_ld<fn>f1<mode>"
+(define_expand "@aarch64_ld<fn>f1<mode>"
+ [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_dup 2)
+ (match_dup 1)
+ ] UNSPEC_UPDATE_FFR)
+ )
+ (set (match_operand:SVE_FULL 0 "register_operand")
+ (unspec:SVE_FULL
+ [(match_operand:<VPRED> 2 "register_operand")
+ (match_operand:SVE_FULL 1 "aarch64_sve_ld<fn>f1_operand")
+ (reg:VNx16BI FFRT_REGNUM)]
+ SVE_LDFF1_LDNF1))]
+ "TARGET_SVE && TARGET_NON_STREAMING"
+ {}
+)
+
+(define_insn "*aarch64_ld<fn>f1<mode>"
[(set (match_operand:SVE_FULL 0 "register_operand" "=w")
(unspec:SVE_FULL
[(match_operand:<VPRED> 2 "register_operand" "Upl")
@@ -1479,7 +1519,36 @@ (define_insn "@aarch64_ld<fn>f1<mode>"
;; -------------------------------------------------------------------------
;; Predicated first-faulting or non-faulting load and extend.
-(define_insn_and_rewrite "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_I:mode>"
+(define_expand "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_I:mode>"
+ [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_dup 2)
+ (match_dup 1)
+ ] UNSPEC_UPDATE_FFR)
+ )
+ (set (match_operand:SVE_HSDI 0 "register_operand")
+ (unspec:SVE_HSDI
+ [(match_operand:<SVE_HSDI:VPRED> 3 "general_operand")
+ (ANY_EXTEND:SVE_HSDI
+ (unspec:SVE_PARTIAL_I
+ [(match_operand:<SVE_PARTIAL_I:VPRED> 2 "register_operand")
+ (match_operand:SVE_PARTIAL_I 1 "aarch64_sve_ld<fn>f1_operand")
+ (reg:VNx16BI FFRT_REGNUM)]
+ SVE_LDFF1_LDNF1))]
+ UNSPEC_PRED_X))]
+ "TARGET_SVE
+ && TARGET_NON_STREAMING
+ && (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
+ {
+ if (!CONSTANT_P (operands[3]))
+ operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
+ }
+)
+
+;; Predicated first-faulting or non-faulting load and extend.
+(define_insn "*aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SVE_PARTIAL_I:mode>"
[(set (match_operand:SVE_HSDI 0 "register_operand" "=w")
(unspec:SVE_HSDI
[(match_operand:<SVE_HSDI:VPRED> 3 "general_operand" "UplDnm")
@@ -1494,10 +1563,6 @@ (define_insn_and_rewrite "@aarch64_ld<fn>f1_<ANY_EXTEND:optab><SVE_HSDI:mode><SV
&& TARGET_NON_STREAMING
&& (~<SVE_HSDI:narrower_mask> & <SVE_PARTIAL_I:self_mask>) == 0"
"ld<fn>f1<ANY_EXTEND:s><SVE_PARTIAL_I:Vesize>\t%0.<SVE_HSDI:Vctype>, %2/z, %1"
- "&& !CONSTANT_P (operands[3])"
- {
- operands[3] = CONSTM1_RTX (<SVE_HSDI:VPRED>mode);
- }
[(set_attr "sve_type" "sve_load_1reg")]
)
@@ -1907,7 +1972,33 @@ (define_insn_and_rewrite "*aarch64_gather_load_<ANY_EXTEND:optab><SVE_2HSDI:mode
;; Predicated first-faulting gather loads for 32-bit elements. Operand
;; 3 is true for unsigned extension and false for signed extension.
-(define_insn "@aarch64_ldff1_gather<mode>"
+(define_expand "@aarch64_ldff1_gather<mode>"
+ [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_dup 5)
+ (match_dup 1)
+ (match_dup 2)
+ (match_dup 3)
+ (match_dup 4)
+ ] UNSPEC_UPDATE_FFR)
+ )
+ (set (match_operand:SVE_FULL_S 0 "register_operand") ; The actual load
+ (unspec:SVE_FULL_S
+ [(match_operand:VNx4BI 5 "register_operand")
+ (match_operand:DI 1 "aarch64_sve_gather_offset_w")
+ (match_operand:VNx4SI 2 "register_operand")
+ (match_operand:DI 3 "const_int_operand")
+ (match_operand:DI 4 "aarch64_gather_scale_operand_w")
+ (mem:BLK (scratch))
+ (reg:VNx16BI FFRT_REGNUM)]
+ UNSPEC_LDFF1_GATHER))]
+ "TARGET_SVE && TARGET_NON_STREAMING"
+ {}
+)
+
+(define_insn "*aarch64_ldff1_gather<mode>"
[(set (match_operand:SVE_FULL_S 0 "register_operand")
(unspec:SVE_FULL_S
[(match_operand:VNx4BI 5 "register_operand")
@@ -1938,7 +2029,33 @@ (define_insn "@aarch64_ldff1_gather<mode>"
;; Predicated first-faulting gather loads for 64-bit elements. The value
;; of operand 3 doesn't matter in this case.
-(define_insn "@aarch64_ldff1_gather<mode>"
+(define_expand "@aarch64_ldff1_gather<mode>"
+ [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_dup 5)
+ (match_dup 1)
+ (match_dup 2)
+ (match_dup 3)
+ (match_dup 4)
+ ] UNSPEC_UPDATE_FFR)
+ )
+ (set (match_operand:SVE_FULL_D 0 "register_operand")
+ (unspec:SVE_FULL_D
+ [(match_operand:VNx2BI 5 "register_operand")
+ (match_operand:DI 1 "aarch64_sve_gather_offset_d")
+ (match_operand:VNx2DI 2 "register_operand")
+ (match_operand:DI 3 "const_int_operand")
+ (match_operand:DI 4 "aarch64_gather_scale_operand_d")
+ (mem:BLK (scratch))
+ (reg:VNx16BI FFRT_REGNUM)]
+ UNSPEC_LDFF1_GATHER))]
+ "TARGET_SVE && TARGET_NON_STREAMING"
+ {}
+)
+
+(define_insn "*aarch64_ldff1_gather<mode>"
[(set (match_operand:SVE_FULL_D 0 "register_operand")
(unspec:SVE_FULL_D
[(match_operand:VNx2BI 5 "register_operand")
@@ -2032,7 +2149,40 @@ (define_insn "*aarch64_ldff1_gather<mode>_uxtw"
;; Predicated extending first-faulting gather loads for 32-bit elements.
;; Operand 3 is true for unsigned extension and false for signed extension.
-(define_insn_and_rewrite "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_NARROW:mode>"
+(define_expand "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_NARROW:mode>"
+ [(set (reg:VNx16BI FFR_REGNUM) ; Update the FFR
+ (unspec:VNx16BI
+ [(reg:VNx16BI FFRT_REGNUM)
+ (reg:VNx16BI FFR_REGNUM)
+ (match_dup 5)
+ (match_dup 1)
+ (match_dup 2)
+ (match_dup 3)
+ (match_dup 4)
+ ] UNSPEC_UPDATE_FFR)
+ )
+ (set (match_operand:VNx4_WIDE 0 "register_operand")
+ (unspec:VNx4_WIDE
+ [(match_operand:VNx4BI 6 "general_operand")
+ (ANY_EXTEND:VNx4_WIDE
+ (unspec:VNx4_NARROW
+ [(match_operand:VNx4BI 5 "register_operand")
+ (match_operand:DI 1 "aarch64_sve_gather_offset_<VNx4_NARROW:Vesize>")
+ (match_operand:VNx4_WIDE 2 "register_operand")
+ (match_operand:DI 3 "const_int_operand")
+ (match_operand:DI 4 "aarch64_gather_scale_operand_<VNx4_NARROW:Vesize>")
+ (mem:BLK (scratch))
+ (reg:VNx16BI FFRT_REGNUM)]
+ UNSPEC_LDFF1_GATHER))]
+ UNSPEC_PRED_X))]
+ "TARGET_SVE && TARGET_NON_STREAMING"
+ {
+ if (!CONSTANT_P (operands[6]))
+ operands[6] = CONSTM1_RTX (VNx4BImode);
+ }
+)
+
+(define_insn "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mode><VNx4_NARROW:mode>"
[(set (match_operand:VNx4_WIDE 0 "register_operand")
(unspec:VNx4_WIDE
[(match_operand:VNx4BI 6 "general_operand")
@@ -2062,16 +2212,34 @@ (define_insn_and_rewrite "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx4_WIDE:mod
[&w, rk, w, Ui1, i, Upl, UplDnm] ldff1<ANY_EXTEND:s><VNx4_NARROW:Vesize>\t%0.s, %5/z, [%1, %2.s, uxtw %p4]
[?w, rk, 0, Ui1, i, Upl, UplDnm] ^
}
- "&& !CONSTANT_P (operands[6])"
- {
- operands[6] = CONSTM1_RTX (VNx4BImode);
- }
[(set_attr "sve_type" "sve_gatherload_32")]
)
;; Predicated extending first-faulting gather loads for 64-bit elements.
;; The value of operand 3 doesn't matter in this case.
-(define_insn_and_rewrite "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_NARROW:mode>"
+(define_expand "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_NARROW:mode>"
+ [(set (match_operand:VNx2_WIDE 0 "register_operand")
+ (unspec:VNx2_WIDE
+ [(match_operand:VNx2BI 6 "general_operand")
+ (ANY_EXTEND:VNx2_WIDE
+ (unspec:VNx2_NARROW
+ [(match_operand:VNx2BI 5 "register_operand")
+ (match_operand:DI 1 "aarch64_sve_gather_offset_<VNx2_NARROW:Vesize>")
+ (match_operand:VNx2_WIDE 2 "register_operand")
+ (match_operand:DI 3 "const_int_operand")
+ (match_operand:DI 4 "aarch64_gather_scale_operand_<VNx2_NARROW:Vesize>")
+ (mem:BLK (scratch))
+ (reg:VNx16BI FFRT_REGNUM)]
+ UNSPEC_LDFF1_GATHER))]
+ UNSPEC_PRED_X))]
+ "TARGET_SVE && TARGET_NON_STREAMING"
+ {
+ if (!CONSTANT_P (operands[6]))
+ operands[6] = CONSTM1_RTX (VNx2BImode);
+ }
+)
+
+(define_insn "*aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mode><VNx2_NARROW:mode>"
[(set (match_operand:VNx2_WIDE 0 "register_operand")
(unspec:VNx2_WIDE
[(match_operand:VNx2BI 6 "general_operand")
@@ -2097,10 +2265,6 @@ (define_insn_and_rewrite "@aarch64_ldff1_gather_<ANY_EXTEND:optab><VNx2_WIDE:mod
[&w, rk, w, i, i, Upl, UplDnm] ldff1<ANY_EXTEND:s><VNx2_NARROW:Vesize>\t%0.d, %5/z, [%1, %2.d, lsl %p4]
[?w, rk, w, i, i, Upl, UplDnm] ^
}
- "&& !CONSTANT_P (operands[6])"
- {
- operands[6] = CONSTM1_RTX (VNx2BImode);
- }
[(set_attr "sve_type" "sve_gatherload_64")]
)
new file mode 100644
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+#include <arm_sve.h>
+
+ void
+two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
+ unsigned long *r2, unsigned char *o1, unsigned char *o2)
+{
+ svbool_t pt = svptrue_b8 ();
+ svsetffr ();
+ svuint8_t a = svldff1_u8 (pt, p);
+ unsigned long n1 = svcntp_b8 (pt, svrdffr ());
+ svsetffr ();
+ svuint8_t b = svldff1_u8 (pt, q);
+ unsigned long n2 = svcntp_b8 (pt, svrdffr ());
+ svst1_u8 (pt, o1, a);
+ svst1_u8 (pt, o2, b);
+ *r1 = n1;
+ *r2 = n2;
+}
+
+/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
new file mode 100644
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+#include <arm_sve.h>
+
+ void
+two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
+ unsigned long *r2, unsigned int *o1, unsigned int *o2)
+{
+ svbool_t pt = svptrue_b8 ();
+ svsetffr ();
+ svuint32_t a = svldff1ub_u32 (pt, p);
+ unsigned long n1 = svcntp_b8 (pt, svrdffr ());
+ svsetffr ();
+ svuint32_t b = svldff1ub_u32 (pt, q);
+ unsigned long n2 = svcntp_b8 (pt, svrdffr ());
+ svst1_u32 (pt, o1, a);
+ svst1_u32 (pt, o2, b);
+ *r1 = n1;
+ *r2 = n2;
+}
+
+/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
new file mode 100644
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+#include <arm_sve.h>
+
+ void
+two_scans (unsigned char *p, unsigned char *q, unsigned long *r1,
+ unsigned long *r2, unsigned *o1, unsigned *o2,
+ svuint32_t offset1, svuint32_t offset2)
+{
+ svbool_t pt = svptrue_b8 ();
+ svsetffr ();
+ svuint32_t a = svldff1sb_gather_offset_u32 (pt, p, offset1);
+ unsigned long n1 = svcntp_b8 (pt, svrdffr ());
+ svsetffr ();
+ svuint32_t b = svldff1sb_gather_offset_u32 (pt, q, offset2);
+ unsigned long n2 = svcntp_b8 (pt, svrdffr ());
+ svst1_u32 (pt, o1, a);
+ svst1_u32 (pt, o2, b);
+ *r1 = n1;
+ *r2 = n2;
+}
+
+/* { dg-final { scan-assembler-times {\trdffr} 2 } } */
new file mode 100644
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+#include <arm_sve.h>
+
+ void
+two_scans (svuint32_t p, svuint32_t q, unsigned long *r1,
+ unsigned long *r2, unsigned *o1, unsigned *o2,
+ long int offset1, long int offset2)
+{
+ svbool_t pt = svptrue_b8 ();
+ svsetffr ();
+ svuint32_t a = svldff1sb_gather_u32base_offset_u32 (pt, p, offset1);
+ unsigned long n1 = svcntp_b8 (pt, svrdffr ());
+ svsetffr ();
+ svuint32_t b = svldff1sb_gather_u32base_offset_u32 (pt, q, offset2);
+ unsigned long n2 = svcntp_b8 (pt, svrdffr ());
+ svst1_u32 (pt, o1, a);
+ svst1_u32 (pt, o2, b);
+ *r1 = n1;
+ *r2 = n2;
+}
+
+/* { dg-final { scan-assembler-times {\trdffr} 2 } } */