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
> + }
> +)
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>
> +
> (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+.
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
