> -----Original Message-----
> From: Artemiy Volkov <[email protected]>
> Sent: 04 September 2026 13:43
> To: Tamar Christina <[email protected]>
> Cc: [email protected]; [email protected]; Richard Earnshaw
> <[email protected]>; [email protected]
> Subject: Re: [PATCH] aarch64: fix combinez patterns for FP16 types
> [PR126983]
>
> On Thu, Sep 03, 2026 at 01:09:46PM +0100, Tamar Christina wrote:
> > Hi Artemiy,
> >
> > > -----Original Message-----
> > > From: Artemiy Volkov <[email protected]>
> > > Sent: 27 August 2026 15:25
> > > To: [email protected]
> > > Cc: [email protected]; Richard Earnshaw
> <[email protected]>;
> > > Tamar Christina <[email protected]>;
> > > [email protected]; Artemiy Volkov
> > > <[email protected]>
> > > Subject: [PATCH] aarch64: fix combinez patterns for FP16 types
> [PR126983]
> > >
> > > The combinez patterns for scalar and partial vector operands added in
> > > r17-898-g920eeb67a3537b use the FMOV instruction indiscriminately,
> > > including for scalar FP16 modes in absence of the fp16 extension, causing
> > > the assemble-time failure reported in PR126983. Fix this by specializing
> > > the
> > > patterns for HF and BF modes and using the indexed INS instruction as
> > > fallback.
> > >
> > > This patch also adds the new vec-init-23-nofp16.c test, which simply tries
> > > to assemble the testcases from vec-init-23.c without the fp16 extension
> > > (including test_float16_6 that triggers the problem), as well as adjusts
> > > expected codegen for one testcase in sve/vec_init_5.c.
> > >
> > > Bootstrapped and regtested on aarch64-linux-gnu.
> > >
> > > gcc/ChangeLog:
> > >
> > > * config/aarch64/aarch64-simd.md
> > > (*aarch64_combine_internal<mode>): Adjust to use the new
> > > VS32_I_VSSUB64_F iterator.
> > > (*aarch64_combine_internal_be<mode>): Likewise.
> > > (*aarch64_combinez<mode>): Split into FP16 and non-FP16 patterns.
> > > (*aarch64_combinez_be<mode): Likewise.
> > > * config/aarch64/iterators.md: (VSSUB64_F): Remove iterator.
> > > (VS32_I_SUB64_F): Likewise.
> > > (VSUB64_F): New iterator.
> > > (VS32_I_VSUB64_F): Likewise.
> > > (VS32_I_VSSUB64_F): Likewise.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/aarch64/sve/vec_init_5.c: Adjust testcase.
> > > * gcc.target/aarch64/vec-init-23-nofp16.c: New test.
> > > ---
> > > gcc/config/aarch64/aarch64-simd.md | 49 ++++++++++++++++---
> > > gcc/config/aarch64/iterators.md | 9 ++--
> > > .../gcc.target/aarch64/sve/vec_init_5.c | 6 +--
> > > .../gcc.target/aarch64/vec-init-23-nofp16.c | 4 ++
> > > 4 files changed, 54 insertions(+), 14 deletions(-)
> > > create mode 100644 gcc/testsuite/gcc.target/aarch64/vec-init-23-
> nofp16.c
> > >
> > > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > > b/gcc/config/aarch64/aarch64-simd.md
> > > index f9b2fd6b2db..98f5ffb54b8 100644
> > > --- a/gcc/config/aarch64/aarch64-simd.md
> > > +++ b/gcc/config/aarch64/aarch64-simd.md
> > > @@ -5003,8 +5003,8 @@
> > > (define_insn "*aarch64_combine_internal<mode>"
> > > [(set (match_operand:<VDBL> 0 "register_operand")
> > > (vec_concat:<VDBL>
> > > - (match_operand:VS32_I_SUB64_F 1 "register_operand")
> > > - (match_operand:VS32_I_SUB64_F 2
> > > "aarch64_simd_nonimmediate_operand")))]
> > > + (match_operand:VS32_I_VSSUB64_F 1 "register_operand")
> > > + (match_operand:VS32_I_VSSUB64_F 2
> > > "aarch64_simd_nonimmediate_operand")))]
> > > "TARGET_FLOAT
> > > && !BYTES_BIG_ENDIAN"
> > > {@ [ cons: =0 , 1 , 2 ; attrs: type , arch ]
> > > @@ -5050,8 +5050,8 @@
> > > (define_insn "*aarch64_combine_internal_be<mode>"
> > > [(set (match_operand:<VDBL> 0 "register_operand")
> > > (vec_concat:<VDBL>
> > > - (match_operand:VS32_I_SUB64_F 2
> > > "aarch64_simd_nonimmediate_operand")
> > > - (match_operand:VS32_I_SUB64_F 1 "register_operand")))]
> > > + (match_operand:VS32_I_VSSUB64_F 2
> > > "aarch64_simd_nonimmediate_operand")
> > > + (match_operand:VS32_I_VSSUB64_F 1 "register_operand")))]
> > > "TARGET_FLOAT
> > > && BYTES_BIG_ENDIAN"
> > > {@ [ cons: =0 , 1 , 2 ; attrs: type , arch ]
> > > @@ -5107,8 +5107,8 @@
> > > (define_insn "*aarch64_combinez<mode>"
> > > [(set (match_operand:<VDBL> 0 "register_operand")
> > > (vec_concat:<VDBL>
> > > - (match_operand:VS32_I_SUB64_F 1 "nonimmediate_operand")
> > > - (match_operand:VS32_I_SUB64_F 2
> > > "aarch64_simd_or_scalar_imm_zero")))]
> > > + (match_operand:VS32_I_VSUB64_F 1 "nonimmediate_operand")
> > > + (match_operand:VS32_I_VSUB64_F 2
> > > "aarch64_simd_or_scalar_imm_zero")))]
> > > "TARGET_FLOAT && !BYTES_BIG_ENDIAN"
> > > {@ [ cons: =0 , 1 ; attrs: type ]
> > > [ w , w ; neon_move ] fmov\t%<single_type>0,
> > > %<single_type>1
> > > @@ -5119,6 +5119,22 @@
> > > }
> > > )
> > >
> > > +(define_insn "*aarch64_combinez<mode>"
> > > + [(set (match_operand:<VDBL> 0 "register_operand")
> > > + (vec_concat:<VDBL>
> > > + (match_operand:HFBF 1 "nonimmediate_operand")
> > > + (match_operand:HFBF 2 "aarch64_simd_or_scalar_imm_zero")))]
> > > + "TARGET_FLOAT && !BYTES_BIG_ENDIAN"
> > > + {@ [ cons: =0 , 1 ; attrs: type, arch ]
> > > + [ w , w ; neon_move , fp16 ] fmov\t%<single_type>0,
> > > %<single_type>1
> > > + [ w , r ; neon_from_gp , fp16 ] fmov\t%<single_type>0,
> > > %<single_wx>1
> > > + [ w , 0 ; neon_move , simd ]
> > > ins\t%0.<single_type>[1], wzr
> > > + [ w , m ; neon_load1_1reg , simd ] ldr\t%<single_type>0, %1
> > > + [ r , r ; mov_reg , * ] uxtw\t%x0, %w1
> > > + [ r , m ; load_4 , * ]
> > > ldr<size>\t%<single_wx>0, %1
> > > + }
> > > +)
>
> Hi Tamar, thanks for taking a look,
>
> >
> > These new patterns look a bit broad though since for TARGET_FLOAT &&
> !TARGET_SIMD_F16INST
> > you now don't have w <- w or w <- r alternatives and would likely cause a
> reload since the predicate
> > becomes wider than the constraints.
> >
> > I think it would be better to keep the original iterators and patterns but
> > add
> constraints to them
> >
> > (define_mode_iterator VSSUB64_F [V2HF V2BF
> > (HF "TARGET_SIMD_F16INST")
> > (BF "TARGET_SIMD_F16INST")])
> > (define_mode_iterator VS32_I_SUB64_F [V4QI V2HI VSSUB64_F])
> >
> > And then just provide new patterns for the fallbacks
> >
> > (define_insn "*aarch64_combine_internal<mode>"
> > [(set (match_operand:<VDBL> 0 "register_operand")
> > (vec_concat:<VDBL>
> > (match_operand:HFBF 1 "register_operand")
> > (match_operand:HFBF 2 "aarch64_simd_nonimmediate_operand")))]
> > "TARGET_FLOAT && TARGET_SIMD && !TARGET_SIMD_F16INST &&
> !BYTES_BIG_ENDIAN"
> > {@ [ cons: =0 , 1 , 2 ; attrs: type , arch ]
> > [ w , w , w ; neon_permute , simd ]
> > zip1\t%0.<Vdduptype>,
> %1.<Vdduptype>, %2.<Vdduptype>
> > [ w , 0 , w ; neon_move , simd ]
> > mov\t%0.<single_type>[1],
> %2.<single_type>[0]
> > [ w , 0 , Utv ; neon_load1_one_lane , simd ]
> ld1\t{%0.<single_type>}[1], %2
> > [ w , 0 , r ; neon_from_gp , simd ]
> > ins\t%0.<single_type>[1],
> %<single_wx>2
> > }
> > )
> >
> > And similar for aarch64_combinez<mode>
>
> I understand your logic, but based on the HW instructions used by the
> patterns, there is no need to change aarch64_combine_internal since it
> doesn't use FMOV anywhere and so will work for simd && !fp16. Or do you
> think it is worth doing for symmetry with aarch64_combinez?
>
Yeah I was trying to prevent more iterators as they're kinda hard to find today
already.
> Changing only aarch64_combinez as I'm proposing means that we need to
> add
> some new iterators; I could do that the way you suggested, adding something
> like:
>
> (define_mode_iterator VSSUB64_F_NO_FP16 [V2HF V2BF
> (HF "TARGET_SIMD_F16INST")
> (BF "TARGET_SIMD_F16INST")])
>
> and changing aarch64_combinez accordingly. What do you think?
>
Yeah that's fine too. Gives less patterns over a small iterator. So I'm ok
with that.
Thanks,
Tamar
> >
> > > +
> > > (define_insn "*aarch64_combinez_be<mode>"
> > > [(set (match_operand:<VDBL> 0 "register_operand")
> > > (vec_concat:<VDBL>
> > > @@ -5147,8 +5163,8 @@
> > > (define_insn "*aarch64_combinez_be<mode>"
> > > [(set (match_operand:<VDBL> 0 "register_operand")
> > > (vec_concat:<VDBL>
> > > - (match_operand:VS32_I_SUB64_F 2
> > > "aarch64_simd_or_scalar_imm_zero")
> > > - (match_operand:VS32_I_SUB64_F 1 "nonimmediate_operand")))]
> > > + (match_operand:VS32_I_VSUB64_F 2
> > > "aarch64_simd_or_scalar_imm_zero")
> > > + (match_operand:VS32_I_VSUB64_F 1 "nonimmediate_operand")))]
> > > "TARGET_FLOAT && BYTES_BIG_ENDIAN"
> > > {@ [ cons: =0 , 1 ; attrs: type ]
> > > [ w , w ; neon_move ] fmov\t%<single_type>0,
> > > %<single_type>1
> > > @@ -5159,6 +5175,23 @@
> > > }
> > > )
> > >
> > > +(define_insn "*aarch64_combinez_be<mode>"
> > > + [(set (match_operand:<VDBL> 0 "register_operand")
> > > + (vec_concat:<VDBL>
> > > + (match_operand:HFBF 2 "aarch64_simd_or_scalar_imm_zero")
> > > + (match_operand:HFBF 1 "nonimmediate_operand")))]
> > > + "TARGET_FLOAT && BYTES_BIG_ENDIAN"
> > > + {@ [ cons: =0 , 1 ; attrs: type, arch ]
> > > + [ w , w ; neon_move , fp16 ] fmov\t%<single_type>0,
> > > %<single_type>1
> > > + [ w , r ; neon_from_gp , fp16 ] fmov\t%<single_type>0,
> > > %<single_wx>1
> > > + [ w , 0 ; neon_move , simd ]
> > > ins\t%0.<single_type>[1], wzr
> > > + [ w , m ; neon_load1_1reg , simd ] ldr\t%<single_type>0, %1
> > > + [ r , r ; mov_reg , * ] uxtw\t%x0, %w1
> > > + [ r , m ; load_4 , * ]
> > > ldr<size>\t%<single_wx>0, %1
> >
> > Also are these right? HFBF -> <VDBL> means creating a 32-bit vector from
> > 16-bit data.
> >
> > uxtw would zero extend from 32-bit to 64 but your input vector elements
> should be 16-bits.
> > So I think you want uxth and ldrh here since you're zeroing bit 16+.
>
> Yes of course, that should be uxt<size> in the (r,r) alternatives throughout.
>
> Thanks,
> Artemiy
>
> >
> > Thanks,
> > Tamar
> >
> > > + }
> > > +)
> > > +
> > > +
> > > ;; Form a vector whose first half (in array order) comes from operand 1
> > > ;; and whose second half (in array order) comes from operand 2.
> > > ;; This operand order follows the RTL vec_concat operation.
> > > diff --git a/gcc/config/aarch64/iterators.md
> > > b/gcc/config/aarch64/iterators.md
> > > index b0571460ec7..296faf6bb2e 100644
> > > --- a/gcc/config/aarch64/iterators.md
> > > +++ b/gcc/config/aarch64/iterators.md
> > > @@ -253,11 +253,14 @@
> > > ;; All sub-32-bit integer modes.
> > > (define_mode_iterator VSSUB32_I [V2QI QI HI])
> > >
> > > -;; All sub-64-bit floating-point modes.
> > > -(define_mode_iterator VSSUB64_F [V2HF V2BF HF BF])
> > > +;; All sub-64-bit vector floating-point modes.
> > > +(define_mode_iterator VSUB64_F [V2HF V2BF])
> > > +
> > > +;; All 32-bit integer and vector sub-64-bit floating point modes.
> > > +(define_mode_iterator VS32_I_VSUB64_F [V4QI V2HI VSUB64_F])
> > >
> > > ;; All 32-bit integer and sub-64-bit floating point modes.
> > > -(define_mode_iterator VS32_I_SUB64_F [V4QI V2HI VSSUB64_F])
> > > +(define_mode_iterator VS32_I_VSSUB64_F [V4QI V2HI VSUB64_F HFBF])
> > > ;; All Advanced SIMD modes suitable for moving, loading, and storing.
> > > (define_mode_iterator VALL_F16 [V8QI V16QI V4HI V8HI V2SI V4SI V2DI
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/vec_init_5.c
> > > b/gcc/testsuite/gcc.target/aarch64/sve/vec_init_5.c
> > > index 0dd085a9423..d02dfadadb5 100644
> > > --- a/gcc/testsuite/gcc.target/aarch64/sve/vec_init_5.c
> > > +++ b/gcc/testsuite/gcc.target/aarch64/sve/vec_init_5.c
> > > @@ -217,12 +217,12 @@
> > >
> > > /*
> > > ** test_float16_6:
> > > -** fcvt h2, s0
> > > +** fcvt h0, s0
> > > ** fcvt h1, s1
> > > ** fmov h31, 1.0e\+0
> > > -** fmov h2, h2
> > > +** fmov h0, h0
> > > ** zip1 v1\.4h, v1\.4h, v31\.4h
> > > -** dup v0\.2s, v2\.s\[0\]
> > > +** dup v0\.2s, v0\.s\[0\]
> > > ** dup v1\.2s, v1\.s\[0\]
> > > ** zip1 v0\.8h, v0\.8h, v1\.8h
> > > ** dup z0\.q, z0\.q\[0\]
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/vec-init-23-nofp16.c
> > > b/gcc/testsuite/gcc.target/aarch64/vec-init-23-nofp16.c
> > > new file mode 100644
> > > index 00000000000..636f4758c19
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/vec-init-23-nofp16.c
> > > @@ -0,0 +1,4 @@
> > > +/* { dg-do assemble } */
> > > +/* { dg-options "-O2 -march=armv8-a" } */
> > > +
> > > +#include "vec-init-23.c"
> > > --
> > > 2.43.0
> >
