Hi Alfie,
Sorry for the long wait on this one. Comments inline below.
On 24/10/2025 21:11, Alfie Richards wrote:
> Hi All,
>
> Embarrassingly, I had run the regression tests for V1, but apparently not
> looked at the results.
>
> Sorry about that, here's a not totally broken version.
>
> Regression tested (properly) for AArch64.
>
> Okay for master?
>
> Thanks,
> Alfie
>
> -- >8 --
>
> Adds support for the AArch64 2024 fmmla extensions.
>
> Note this includes a work around for spurious warnings from binutils with
> movprfx and fmmla instructions.
> (PR gas/33562).
Please can you clarify here that the workaround is just in the testsuite
and not an actual codegen workaround.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-builtins.cc (aarch64_expand_pragma_builtin):
> Add case for FMMLA.
> * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins):
> Add new __ARM_FEATURE_X macros.
> * config/aarch64/aarch64-option-extensions.def (AARCH64_OPT_EXTENSION):
> Add f8f16mm, f8f32mm, and sve-f16f32mm extensions.
> * config/aarch64/aarch64-simd-pragma-builtins.def (vmmlaq_f16_mf8):
> New intrinsic.
> (vmmlaq_f32_mf8): New intrinsic.
> * config/aarch64/aarch64-simd.md (@aarch64_<insn><V8HF_ONLY:mode>): New
> instruction.
> (@aarch64_<insn><V4SF_ONLY:mode>): New instruction.
> * config/aarch64/aarch64-sve-builtins-base.cc: Update mmla_impl for
> new instructions.
> * config/aarch64/aarch64-sve-builtins-shapes.cc (struct mmla_def): Add
> support for the new widening forms.
> * config/aarch64/aarch64-sve-builtins-sve2.def (svmmla) Add new
> intrinsics.
> * config/aarch64/aarch64-sve-builtins.cc (TYPES_cvt_narrow_s): Fix
> comment.
> * config/aarch64/aarch64-sve2.md (aarch64_sve_fmmlavnx8hfvnx16qi): New
> instruction.
> (aarch64_sve_fmmlavnx4sfvnx16qi): New instruction.
> (aarch64_sve_fmmlavnx4sfvnx8hf): New instruction.
> * config/aarch64/aarch64.h (TARGET_F8F32MM): New macro.
> (TARGET_F8F16MM): New macro.
> (TARGET_SVE_F16F32MM): New macro.
> * config/aarch64/iterators.md (insn): Add fmmla entry.
>
> gcc/testsuite/ChangeLog:
>
> * lib/target-supports.exp:
> * gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f16_mf8.c: New test.
> * gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f32_mf8.c: New test.
> * gcc.target/aarch64/sve2/acle/asm/fmmla_f8f16mm_sve2.c: New test.
> * gcc.target/aarch64/sve2/acle/asm/fmmla_f8f32mm_sve2.c: New test.
> * gcc.target/aarch64/sve2/acle/asm/fmmla_sve_f16f32mm.c: New test.
> * gcc.target/aarch64/sve/acle/general-c/mmla_1.c: Update error messages.
> ---
> gcc/config/aarch64/aarch64-builtins.cc | 1 +
> gcc/config/aarch64/aarch64-c.cc | 5 ++
> .../aarch64/aarch64-option-extensions.def | 6 ++
> .../aarch64/aarch64-simd-pragma-builtins.def | 10 +++
> gcc/config/aarch64/aarch64-simd.md | 29 +++++++++
> .../aarch64/aarch64-sve-builtins-base.cc | 19 +++++-
> .../aarch64/aarch64-sve-builtins-shapes.cc | 50 +++++++++++----
> .../aarch64/aarch64-sve-builtins-sve2.def | 15 +++++
> gcc/config/aarch64/aarch64-sve-builtins.cc | 2 +-
> gcc/config/aarch64/aarch64-sve2.md | 62 +++++++++++++++++++
> gcc/config/aarch64/aarch64.h | 7 +++
> gcc/config/aarch64/iterators.md | 5 +-
> .../advsimd-intrinsics/vmmlaq_f16_mf8.c | 32 ++++++++++
> .../advsimd-intrinsics/vmmlaq_f32_mf8.c | 32 ++++++++++
> .../aarch64/sve/acle/general-c/mmla_1.c | 16 ++---
> .../sve2/acle/asm/fmmla_f8f16mm_sve2.c | 33 ++++++++++
> .../sve2/acle/asm/fmmla_f8f32mm_sve2.c | 33 ++++++++++
> .../sve2/acle/asm/fmmla_sve_f16f32mm.c | 31 ++++++++++
> gcc/testsuite/lib/target-supports.exp | 2 +-
> 19 files changed, 365 insertions(+), 25 deletions(-)
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f16_mf8.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f32_mf8.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f16mm_sve2.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f32mm_sve2.c
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_sve_f16f32mm.c
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.cc
> b/gcc/config/aarch64/aarch64-builtins.cc
> index 408099a50e8..8e8ba035705 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -4009,6 +4009,7 @@ aarch64_expand_pragma_builtin (tree exp, rtx target,
>
> case UNSPEC_FAMAX:
> case UNSPEC_FAMIN:
> + case UNSPEC_FMMLA:
> case UNSPEC_F1CVTL_FP8:
> case UNSPEC_F2CVTL_FP8:
> case UNSPEC_FDOT_FP8:
> diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
> index c3957c762ef..824dd423250 100644
> --- a/gcc/config/aarch64/aarch64-c.cc
> +++ b/gcc/config/aarch64/aarch64-c.cc
> @@ -309,6 +309,11 @@ aarch64_update_cpp_builtins (cpp_reader *pfile)
> cpp_undef (pfile, "__FLT_EVAL_METHOD_C99__");
> builtin_define_with_int_value ("__FLT_EVAL_METHOD_C99__",
> c_flt_eval_method (false));
> +
> + aarch64_def_or_undef (TARGET_F8F16MM, "__ARM_FEATURE_F8F16MM", pfile);
> + aarch64_def_or_undef (TARGET_F8F32MM, "__ARM_FEATURE_F8F32MM", pfile);
> + aarch64_def_or_undef (TARGET_SVE_F16F32MM, "__ARM_FEATURE_SVE_F16F32MM",
> + pfile);
> }
>
> /* Implement TARGET_CPU_CPP_BUILTINS. */
> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def
> b/gcc/config/aarch64/aarch64-option-extensions.def
> index a70375c053f..61755da91e2 100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -279,6 +279,12 @@ AARCH64_OPT_EXTENSION("lut", LUT, (SIMD), (), (), "lut")
>
> AARCH64_OPT_EXTENSION ("sme-lutv2", SME_LUTv2, (SME2), (), (), "sme-lutv2")
>
> +AARCH64_OPT_EXTENSION ("f8f16mm", F8F16MM, (SIMD, FP8), (), (), "")
> +
> +AARCH64_OPT_EXTENSION ("f8f32mm", F8F32MM, (SIMD, FP8), (), (), "")
> +
> +AARCH64_OPT_EXTENSION ("sve-f16f32mm", SVE_F16F32MM, (SVE), (), (), "")
> +
> AARCH64_OPT_EXTENSION("cpa", CPA, (), (), (), "")
>
> #undef AARCH64_OPT_FMV_EXTENSION
> diff --git a/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
> b/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
> index 77682365103..5e7eefecca8 100644
> --- a/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
> +++ b/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
> @@ -296,6 +296,16 @@ ENTRY_LOAD_LANE (vld4q_lane_mf8, mf8qx4,
> mf8_scalar_const_ptr, mf8qx4,
> UNSPEC_LD4_LANE)
> #undef REQUIRED_EXTENSIONS
>
> +// mmla f16 mf8
> +#define REQUIRED_EXTENSIONS nonstreaming_only (TARGET_SIMD | TARGET_F8F16MM)
nonstreaming_only takes an argument of type aarch64_feature_flags, so we need to
use the AARCH64_FL_* macros here, not TARGET_*, see PR123206.
Indeed, it leads to an ICE with the following testcase:
$ cat t.c
#include <arm_neon.h>
float16x8_t f(float16x8_t acc, mfloat8x16_t v, mfloat8x16_t w, fpm_t fpm)
{
return vmmlaq_f16_mf8(acc, v, w, fpm);
}
$ ./xgcc -B . -c t.c -S -o /dev/null -O2
t.c: In function âfâ:
t.c:5:1: error: unrecognizable insn:
5 | }
| ^
(insn 10 9 14 2 (set (reg:V8HF 101 [ <retval> ])
(unspec:V8HF [
(reg/v:V16QI 103 [ v ])
(reg/v:V16QI 104 [ w ])
(reg/v:V8HF 102 [ acc ])
(reg:DI 84 fpmr)
] UNSPEC_FMMLA)) "t.c":4:10 -1
(nil))
during RTL pass: vregs
t.c:5:1: internal compiler error: in extract_insn, at recog.cc:2884
[...]
since the pattern (correctly) is only enabled if +f8f16mm is provided.
Also, I don't think you need to explicitly include SIMD here, since it is a
dependency of +f8f16mm. So this can just be AARCH64_FL_F8F16MM.
> +ENTRY_TERNARY (vmmlaq_f16_mf8, f16q, f16q, mf8q, mf8q, UNSPEC_FMMLA, FP8)
> +#undef REQUIRED_EXTENSIONS
> +
> +// mmla f32 mf8
> +#define REQUIRED_EXTENSIONS nonstreaming_only (TARGET_SIMD | TARGET_F8F32MM)
Likewise, this should just be AARCH64_FL_F8F32MM.
> +ENTRY_TERNARY (vmmlaq_f32_mf8, f32q, f32q, mf8q, mf8q, UNSPEC_FMMLA, FP8)
> +#undef REQUIRED_EXTENSIONS
> +
> // mov
> #define REQUIRED_EXTENSIONS nonstreaming_only (TARGET_SIMD)
> ENTRY_UNARY (vmov_n_mf8, mf8, mf8_scalar, UNSPEC_DUP, QUIET)
> diff --git a/gcc/config/aarch64/aarch64-simd.md
> b/gcc/config/aarch64/aarch64-simd.md
> index a121a18f9a0..ce268beecb3 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -10535,3 +10535,32 @@ (define_insn
> "@aarch64_<insn>_lane<V4SF_ONLY:mode><VB:mode>"
> return "<insn>\t%0.<V4SF_ONLY:Vtype>, %2.16b, %3.b[%4]";
> }
> )
> +
> +(define_insn "@aarch64_<insn><V8HF_ONLY:mode>"
> + [(set (match_operand:V8HF 0 "register_operand")
> + (unspec:V8HF_ONLY
> + [(match_operand:V16QI 2 "register_operand")
> + (match_operand:V16QI 3 "register_operand")
> + (match_operand:V8HF 1 "register_operand")
> + (reg:DI FPM_REGNUM)]
> + FMMLA))]
> + "TARGET_F8F16MM"
> + {@ [ cons: =0 , 1 , 2 , 3 ]
> + [ w , 0 , w , w ] <insn>\t%0.8h, %2.16b, %3.16b
> + }
> +)
> +
> +(define_insn "@aarch64_<insn><V4SF_ONLY:mode>"
> + [(set (match_operand:V4SF 0 "register_operand")
> + (unspec:V4SF_ONLY
> + [(match_operand:V16QI 2 "register_operand")
> + (match_operand:V16QI 3 "register_operand")
> + (match_operand:V4SF 1 "register_operand")
> + (reg:DI FPM_REGNUM)]
> + FMMLA))]
> + "TARGET_F8F32MM"
> + {@ [ cons: =0 , 1 , 2 , 3 ]
> + [ w , 0 , w , w ] <insn>\t%0.4s, %2.16b, %3.16b
> + }
> +)
It might be neater to refactor this into a single pattern using a conditional
mode iterator like the following:
(define_mode_iterator VQ_HSF_FMMLA [(V8HF "TARGET_F8F16MM") (V4SF
"TARGET_F8F32MM")])
you should be able to project out the right arrangement specifier using the
Vtype mode attribute.
> +
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index ecc06877cac..2fb19ce9b8e 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -2283,7 +2283,24 @@ public:
> icode = code_for_aarch64_sve_add (UNSPEC_SMATMUL, e.vector_mode (0));
> }
> else
> - icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0));
> + {
> + if (e.type_suffix (0).element_bits == 16
> + && e.type_suffix (1).element_bits == 8)
> + icode = CODE_FOR_aarch64_sve_fmmlavnx8hfvnx16qi;
> + else if (e.type_suffix (0).element_bits == 32
> + && e.type_suffix (1).element_bits == 8)
> + icode = CODE_FOR_aarch64_sve_fmmlavnx4sfvnx16qi;
> + else if (e.type_suffix (0).element_bits == 32
> + && e.type_suffix (1).element_bits == 16)
> + icode = CODE_FOR_aarch64_sve_fmmlavnx4sfvnx8hf;
Can we refactor the new patterns to use @ patterns? It should then be
possible to refactor this expand logic to something like this:
if (e.type_suffix_ids[1] == NUM_TYPE_SUFFIXES)
icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0));
else
icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0),
e.vector_mode (1));
> + else
> + {
> + /* We must be in the case where the mode is a single type, and
> + therefore mode 1 is invalid. */
> + gcc_assert (!e.vector_mode (1));
> + icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0));
> + }
> + }
> return e.use_exact_insn (icode);
> }
> };
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
> index b315dc91cc7..298bd51f139 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
> @@ -1019,7 +1019,8 @@ template <unsigned int BITS> struct luti_zt_base :
> public nonoverloaded_base
>
> /* sv<t0>_t svfoo[_t0](sv<t0>_t, sv<t0:quarter>_t,
> sv<t0:quarter>_t) (for integer t0)
> - sv<t0>_t svmmla[_t0](sv<t0>_t, sv<t0>_t, sv<t0>_t) (for floating-point
> t0)
Shouldn't we keep this line, since we still handle this form for +f{32,64}mm?
> + sv<t0>_t svmmla[_t0](sv<t0>_t, sv<t1>_t, sv<t1>_t)
> + (for floating-point t0, t1)
>
> The functions act like the equivalent of "ternary_qq" for integer elements
> and normal vector-only ternary functions for floating-point elements. */
> @@ -1030,7 +1031,12 @@ struct mmla_def : public overloaded_base<0>
> {
> b.add_overloaded_functions (group, MODE_none);
> if (type_suffixes[group.types[0][0]].float_p)
> - build_all (b, "v0,v0,v0,v0", group, MODE_none);
> + {
> + if (group.types[0][1] == NUM_TYPE_SUFFIXES)
> + build_all (b, "v0,v0,v0,v0", group, MODE_none);
> + else
> + build_all (b, "v0,v0,v1,v1", group, MODE_none);
> + }
> else
> build_all (b, "v0,v0,vq0,vq0", group, MODE_none);
> }
> @@ -1039,24 +1045,42 @@ struct mmla_def : public overloaded_base<0>
> resolve (function_resolver &r) const override
> {
> unsigned int i, nargs;
> - type_suffix_index type;
> - if (!r.check_gp_argument (3, i, nargs)
> - || (type = r.infer_vector_type (i)) == NUM_TYPE_SUFFIXES)
> + type_suffix_index type1;
> + type_suffix_index type2;
Style nit: we might as well declare these on a single line like:
type_suffix_index type1, type2;
> + if (!r.check_gp_argument (3, i, nargs))
> + return error_mark_node;
> +
> + if ((type1 = r.infer_vector_type (i)) == NUM_TYPE_SUFFIXES
> + || (type2 = r.infer_vector_type (i + 1)) == NUM_TYPE_SUFFIXES)
Any reason that you don't just add these || clauses onto the original if as the
original code did?
> return error_mark_node;
>
> + bool float_p = type_suffixes[type1].float_p;
> /* Make sure that the function exists now, since not all forms
> follow a set pattern after this point. */
> - tree res = r.resolve_to (r.mode_suffix_id, type);
> + tree res = (float_p && type1 != type2)
> + ? r.resolve_to (r.mode_suffix_id, type1, type2)
> + : r.resolve_to (r.mode_suffix_id, type1);
> if (res == error_mark_node)
> return res;
>
> - bool float_p = type_suffixes[type].float_p;
> - unsigned int modifier = float_p ? r.SAME_SIZE : r.QUARTER_SIZE;
> - if (!r.require_derived_vector_type (i + 1, i, type, r.SAME_TYPE_CLASS,
> - modifier)
> - || !r.require_derived_vector_type (i + 2, i, type, r.SAME_TYPE_CLASS,
> - modifier))
> - return error_mark_node;
> + if (float_p)
> + {
> + /* In the float case, require arg i+1 to have same type as i+2. */
> + if (!r.require_derived_vector_type (i + 2, i + 1, type2,
> + r.SAME_TYPE_CLASS, r.SAME_SIZE))
> + return error_mark_node;
> + }
> + else
> + {
> + /* In the int case, require arg i+1 and i+2 to have a quater the size
"quarter"
> + of arg i. */
> + if (!r.require_derived_vector_type (i + 1, i, type1, r.SAME_TYPE_CLASS,
> + r.QUARTER_SIZE)
> + || !r.require_derived_vector_type (i + 2, i, type1,
> + r.SAME_TYPE_CLASS,
> + r.QUARTER_SIZE))
> + return error_mark_node;
> + }
>
> return res;
> }
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> index b622fe33458..a59a5e2babc 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> @@ -418,3 +418,18 @@ DEF_SVE_FUNCTION_GS_FPM (svdot_lane,
> ternary_mfloat8_lane_group_selection, s_flo
> DEF_SVE_FUNCTION_GS_FPM (svdot, ternary_mfloat8, h_float_mf8, none, none,
> set)
> DEF_SVE_FUNCTION_GS_FPM (svdot_lane, ternary_mfloat8_lane_group_selection,
> h_float_mf8, none, none, set)
> #undef REQUIRED_EXTENSIONS
> +
> +#define REQUIRED_EXTENSIONS \
> + nonstreaming_sve (AARCH64_FL_SVE2 | AARCH64_FL_F8F16MM)
> +DEF_SVE_FUNCTION_GS_FPM (svmmla, mmla, h_float_mf8, none, none, set)
> +#undef REQUIRED_EXTENSIONS
> +
> +#define REQUIRED_EXTENSIONS \
> + nonstreaming_sve (AARCH64_FL_SVE2 | AARCH64_FL_F8F32MM)
> +DEF_SVE_FUNCTION_GS_FPM (svmmla, mmla, s_float_mf8, none, none, set)
> +#undef REQUIRED_EXTENSIONS
> +
> +#define REQUIRED_EXTENSIONS \
> + nonstreaming_sve (AARCH64_FL_SVE_F16F32MM)
Very minor, but no need for the line break here.
> +DEF_SVE_FUNCTION_GS (svmmla, mmla, cvt_f32_f16, none, none)
Also minor, but this can just use DEF_SVE_FUNCTION, no need to use the
GS variant since you have GROUPS = none.
> +#undef REQUIRED_EXTENSIONS
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc
> b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index b2b03dc8cea..b0e262a7c2d 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -478,7 +478,7 @@ CONSTEXPR const group_suffix_info group_suffixes[] = {
> #define TYPES_cvt_long(S, D) \
> D (f32, f16), D (f64, f32)
>
> -/* _f16_f32. */
> +/* _f32_f64. */
> #define TYPES_cvt_narrow_s(S, D) \
> D (f32, f64)
>
> diff --git a/gcc/config/aarch64/aarch64-sve2.md
> b/gcc/config/aarch64/aarch64-sve2.md
> index 91091835182..1d97a98207e 100644
> --- a/gcc/config/aarch64/aarch64-sve2.md
> +++ b/gcc/config/aarch64/aarch64-sve2.md
> @@ -134,6 +134,9 @@
> ;; ---- Optional AES extensions
> ;; ---- Optional SHA-3 extensions
> ;; ---- Optional SM4 extensions
> +;;
> +;; == FMMLA extensions
> +;; ---- [FP] Matrix multiply-accumulate widening
>
> ;; =========================================================================
> ;; == Moves
> @@ -4608,3 +4611,62 @@ (define_insn "aarch64_sve2_sm4ekey"
> "sm4ekey\t%0.s, %1.s, %2.s"
> [(set_attr "type" "crypto_sm4")]
> )
> +
> +;; =========================================================================
> +;; == FMMLA extensions
> +;; =========================================================================
> +
> +;; -------------------------------------------------------------------------
> +;; ---- [FP] Matrix multiply-accumulate widening
> +;; -------------------------------------------------------------------------
> +;; Includes:
> +;; - FMMLA (F8F16MM,F8F32MM,SVE_F16F32MM)
> +;; -------------------------------------------------------------------------
> +
> +(define_insn "aarch64_sve_fmmlavnx8hfvnx16qi"
> + [(set (match_operand:VNx8HF 0 "register_operand")
> + (unspec:VNx8HF
> + [(match_operand:VNx16QI 2 "register_operand")
> + (match_operand:VNx16QI 3 "register_operand")
> + (match_operand:VNx8HF 1 "register_operand")
> + (reg:DI FPM_REGNUM)]
> + FMMLA))]
> + "TARGET_SVE2 && TARGET_F8F16MM"
I think we also need "&& TARGET_NON_STREAMING" here.
> + {@ [ cons: =0 , 1 , 2 , 3 ; attrs: movprfx ]
> + [ w , 0 , w , w ; * ] fmmla\t%0.h, %2.b, %3.b
> + [ ?&w , w , w , w ; yes ] movprfx\t%0, %1\;fmmla\t%0.h,
> %2.b, %3.b
> + }
> + [(set_attr "sve_type" "sve_fp_mul")]
> +)
> +
> +(define_insn "aarch64_sve_fmmlavnx4sfvnx16qi"
> + [(set (match_operand:VNx4SF 0 "register_operand")
> + (unspec:VNx4SF
> + [(match_operand:VNx16QI 2 "register_operand")
> + (match_operand:VNx16QI 3 "register_operand")
> + (match_operand:VNx4SF 1 "register_operand")
> + (reg:DI FPM_REGNUM)]
> + FMMLA))]
> + "TARGET_SVE2 && TARGET_F8F32MM"
Likewise, "&& TARGET_NON_STREAMING".
> + {@ [ cons: =0 , 1 , 2 , 3 ; attrs: movprfx ]
> + [ w , 0 , w , w ; * ] fmmla\t%0.s, %2.b, %3.b
> + [ ?&w , w , w , w ; yes ] movprfx\t%0, %1\;fmmla\t%0.s,
> %2.b, %3.b
> + }
> + [(set_attr "sve_type" "sve_fp_mul")]
> +)
As with the AdvSIMD patterns, can we refactor these two to use conditional mode
iterators and collapse them into a single pattern? Can we also make all of
these patterns into @ patterns to simplify the expansion logic?
> +
> +(define_insn "aarch64_sve_fmmlavnx4sfvnx8hf"
> + [(set (match_operand:VNx4SF 0 "register_operand")
> + (unspec:VNx4SF
> + [(match_operand:VNx8HF 2 "register_operand")
> + (match_operand:VNx8HF 3 "register_operand")
> + (match_operand:VNx4SF 1 "register_operand")]
> + FMMLA))]
> + "TARGET_SVE_F16F32MM"
As above, I think this also needs "&& TARGET_NON_STREAMING".
> + {@ [ cons: =0 , 1 , 2 , 3 ; attrs: movprfx ]
> + [ w , 0 , w , w ; * ] fmmla\t%0.s, %2.h, %3.h
> + [ ?&w , w , w , w ; yes ] movprfx\t%0, %1\;fmmla\t%0.s,
> %2.h, %3.h
> + }
> + [(set_attr "sve_type" "sve_fp_mul")]
> +)
> +
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 2cd929d83f9..2fef81f0dcd 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -392,6 +392,13 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE
> ATTRIBUTE_UNUSED
> but are incompatible with -mtrack-speculation. */
> #define TARGET_CMPBR (AARCH64_HAVE_ISA (CMPBR) && !aarch64_track_speculation)
>
> +/* FP8F32MM instructions, enabled through +f8f32mm. */
"F8F32MM"
> +#define TARGET_F8F32MM (AARCH64_HAVE_ISA (F8F32MM))
> +/* F8F16MM instructions, enabled through +f8f16mm. */
> +#define TARGET_F8F16MM (AARCH64_HAVE_ISA (F8F16MM))
> +/* SVE_F16F32MM instructions, enabled through +sve-f16f32mm. */
> +#define TARGET_SVE_F16F32MM (AARCH64_HAVE_ISA (SVE_F16F32MM))
> +
> /* Make sure this is always defined so we don't have to check for ifdefs
> but rather use normal ifs. */
> #ifndef TARGET_FIX_ERR_A53_835769_DEFAULT
> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> index 517b2808b5f..e2cdfee308c 100644
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -4014,6 +4014,8 @@ (define_int_iterator FSCALE_UNS [UNSPEC_FSCALE])
> (define_int_iterator FPM_FDOT [UNSPEC_FDOT_FP8])
> (define_int_iterator FPM_FDOT_LANE [UNSPEC_FDOT_LANE_FP8])
>
> +(define_int_iterator FMMLA_UNS [UNSPEC_FMMLA])
I think this is dead code, please drop.
> +
> ;; -------------------------------------------------------------------
> ;; Int Iterators Attributes.
> ;; -------------------------------------------------------------------
> @@ -4031,7 +4033,8 @@ (define_int_attr insn
> (UNSPEC_FMLALLBT_FP8 "fmlallbt")
> (UNSPEC_FMLALLTB_FP8 "fmlalltb")
> (UNSPEC_FMLALLTT_FP8 "fmlalltt")
> - (UNSPEC_FSCALE "fscale")])
> + (UNSPEC_FSCALE "fscale")
> + (UNSPEC_FMMLA "fmmla")])
>
> ;; The optab associated with an operation. Note that for ANDF, IORF
> ;; and XORF, the optab pattern is not actually defined; we just use this
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f16_mf8.c
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f16_mf8.c
> new file mode 100644
> index 00000000000..f9af2647419
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f16_mf8.c
> @@ -0,0 +1,32 @@
> +/* { dg-do assemble { target aarch64_asm_f8f16mm_ok } } */
> +/* { dg-do compile { target { ! aarch64_asm_f8f16mm_ok } } } */
> +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
I don't think you want the "-DCHECK_ASM" bit here. As it stands, the
check-function-bodies tests aren't being run at all, because the
advsimd-intrinsics driver doesn't set -DCHECK_ASM. I guess this was
copied from the SVE tests where the driver does pass -DCHECK_ASM.
You're probably better off putting the test in the acle directory, then
you can model your check-function-bodies test off e.g. acle/fp8.c. The
main thing is that you need to drop -DCHECK_ASM, but I don't think
you'll be able to get the test to pass in the advsimd-intrinsics
directory due to all the torture options.
> +
> +#pragma GCC target "+simd+f8f16mm"
I don't think the explicit `+simd` should be necessary here.
> +
> +#include <arm_neon.h>
> +
> +/*
> +** fmmla_f32f8mm_tied:
Wrong function name. This should have been caught if the
check-function-bodies test was actually running.
> +** msr fpmr, x0
> +** fmmla v0\.h, v4\.b, v5\.b
Also, this codegen shouldn't be possible given the test only uses
v0-v2.
> +** ret
> +*/
> +float16x8_t
> +fmmla_f16f8mm_tied (float16x8_t z0, mfloat8x16_t z1, mfloat8x16_t z2, fpm_t
> fpm0)
> +{
> + return vmmlaq_f16_mf8 (z0, z1, z2, fpm0);
> +}
> +
> +/*
> +** svmmla_f16f8mm:
Likewise, wrong function name.
> +** msr fpmr, x0
> +** mov v0, v1
> +** fmmla v0\.h, v4\.b, v5\.b
> +** ret
> +*/
> +float16x8_t
> +fmmla_f16f8mm (float16x8_t z0, float16x8_t z1, mfloat8x16_t z2, mfloat8x16_t
> z3, fpm_t fpm0)
> +{
> + return vmmlaq_f16_mf8 (z1, z2, z3, fpm0);
> +}
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f32_mf8.c
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f32_mf8.c
> new file mode 100644
> index 00000000000..7ed363cbc9b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vmmlaq_f32_mf8.c
> @@ -0,0 +1,32 @@
> +/* { dg-do assemble { target aarch64_asm_f8f32mm_ok } } */
> +/* { dg-do compile { target { ! aarch64_asm_f8f32mm_ok } } } */
> +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
Same comment about removing -DCHECK_ASM and moving the test.
> +
> +#pragma GCC target "+simd+f8f32mm"
Drop +simd.
> +
> +#include <arm_neon.h>
> +
> +/*
> +** fmmla_f32f8mm_tied:
> +** msr fpmr, x0
> +** fmmla v0\.s, v4\.b, v5\.b
> +** ret
> +*/
> +float32x4_t
> +fmmla_f32f8mm_tied (float32x4_t z0, mfloat8x16_t z1, mfloat8x16_t z2, fpm_t
> fpm0)
> +{
> + return vmmlaq_f32_mf8 (z0, z1, z2, fpm0);
> +}
> +
> +/*
> +** svmmla_f32f8mm:
Wrong function name.
> +** msr fpmr, x0
> +** mov v0, v1
> +** fmmla v0\.s, v4\.b, v5\.b
> +** ret
> +*/
> +float32x4_t
> +fmmla_f32f8mm (float32x4_t z0, float32x4_t z1, mfloat8x16_t z2, mfloat8x16_t
> z3, fpm_t fpm0)
> +{
> + return vmmlaq_f32_mf8 (z1, z2, z3, fpm0);
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/mmla_1.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/mmla_1.c
> index ca2ab8a6f3f..a23c45c86b1 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/mmla_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/mmla_1.c
> @@ -43,14 +43,14 @@ f2 (svbool_t pg, svint8_t s8, svuint8_t u8, svuint32_t
> u32, svint32_t s32,
> svmmla (u32, s32, s32); /* { dg-error {passing 'svint32_t' instead of the
> expected 'svuint8_t' to argument 2 of 'svmmla', after passing 'svuint32_t' to
> argument 1} } */
> svmmla (u32, u32, u32); /* { dg-error {passing 'svuint32_t' instead of the
> expected 'svuint8_t' to argument 2 of 'svmmla', after passing 'svuint32_t' to
> argument 1} } */
>
> - svmmla (f16, s8, s8); /* { dg-error {'svmmla' has no form that takes
> 'svfloat16_t' arguments} } */
> - svmmla (f32, s8, s8); /* { dg-error {passing 'svint8_t' to argument 2 of
> 'svmmla', but argument 1 had type 'svfloat32_t'} } */
> - svmmla (f32, s32, s32); /* { dg-error {passing 'svint32_t' to argument 2
> of 'svmmla', but argument 1 had type 'svfloat32_t'} } */
> - svmmla (f32, f16, f16); /* { dg-error {passing 'svfloat16_t' to argument 2
> of 'svmmla', but argument 1 had type 'svfloat32_t'} } */
> - svmmla (f64, f16, f16); /* { dg-error {passing 'svfloat16_t' to argument 2
> of 'svmmla', but argument 1 had type 'svfloat64_t'} } */
> - svmmla (f32, f32, f16); /* { dg-error {passing 'svfloat16_t' to argument 3
> of 'svmmla', but argument 1 had type 'svfloat32_t'} } */
> - svmmla (f64, f32, f16); /* { dg-error {passing 'svfloat32_t' to argument 2
> of 'svmmla', but argument 1 had type 'svfloat64_t'} } */
> - svmmla (f64, f64, f16); /* { dg-error {passing 'svfloat16_t' to argument 3
> of 'svmmla', but argument 1 had type 'svfloat64_t'} } */
> + svmmla (f16, s8, s8); /* { dg-error {'svmmla' has no form that takes
> 'svfloat16_t' and 'svint8_t' arguments} } */
> + svmmla (f32, s8, s8); /* { dg-error {'svmmla' has no form that takes
> 'svfloat32_t' and 'svint8_t' arguments} } */
> + svmmla (f32, s32, s32); /* { dg-error {'svmmla' has no form that takes
> 'svfloat32_t' and 'svint32_t' arguments} } */
> + svmmla (f32, f16, f16); /* { dg-error {ACLE function 'svmmla_f32_f16'
> requires ISA extension 'sve-f16f32mm'} } */
> + svmmla (f64, f16, f16); /* { dg-error {'svmmla' has no form that takes
> 'svfloat64_t' and 'svfloat16_t' arguments} } */
> + svmmla (f32, f32, f16); /* { dg-error {passing 'svfloat16_t' to argument 3
> of 'svmmla', but argument 2 had type 'svfloat32_t'} } */
> + svmmla (f64, f32, f16); /* { dg-error {'svmmla' has no form that takes
> 'svfloat64_t' and 'svfloat32_t' arguments} } */
> + svmmla (f64, f64, f16); /* { dg-error {passing 'svfloat16_t' to argument 3
> of 'svmmla', but argument 2 had type 'svfloat64_t'} } */
>
> svmmla (f16, f16, f16); /* { dg-error {'svmmla' has no form that takes
> 'svfloat16_t' arguments} } */
> svmmla (f32, f32, f32);
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f16mm_sve2.c
> b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f16mm_sve2.c
> new file mode 100644
> index 00000000000..9a333ff4b1d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f16mm_sve2.c
> @@ -0,0 +1,33 @@
> +/* { dg-do assemble { target aarch64_asm_f8f16mm_ok } } */
> +/* { dg-do compile { target { ! aarch64_asm_f8f16mm_ok } } } */
> +/* { dg-skip-if "" { *-*-* } { "-DSTREAMING_COMPATIBLE" } { "" } } */
> +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
> +
> +/* Binutils PR gas/33562 */
> +/* { dg-prune-output "SVE `movprfx' compatible instruction expected" } */
> +
> +#include "test_sve_acle.h"
> +
> +#pragma GCC target "+sve2+f8f16mm"
> +
> +/*
> +** svmmla_f16f8mm_tied:
> +** msr fpmr, x0
> +** fmmla z0\.h, z4\.b, z5\.b
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f16f8mm_tied, svfloat16_t, svmfloat8_t,
> + z0 = svmmla_f16_mf8_fpm (z0, z4, z5, fpm0),
> + z0 = svmmla_fpm (z0, z4, z5, fpm0))
> +
> +/*
> +** svmmla_f16f8mm:
> +** msr fpmr, x0
> +** movprfx z0, z1
> +** fmmla z0\.h, z4\.b, z5\.b
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f16f8mm, svfloat16_t, svmfloat8_t,
> + z0 = svmmla_f16_mf8_fpm (z1, z4, z5, fpm0),
> + z0 = svmmla_fpm (z1, z4, z5, fpm0))
> +
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f32mm_sve2.c
> b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f32mm_sve2.c
> new file mode 100644
> index 00000000000..edd8cef3a12
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_f8f32mm_sve2.c
> @@ -0,0 +1,33 @@
> +/* { dg-do assemble { target aarch64_asm_f8f32mm_ok } } */
> +/* { dg-do compile { target { ! aarch64_asm_f8f32mm_ok } } } */
> +/* { dg-skip-if "" { *-*-* } { "-DSTREAMING_COMPATIBLE" } { "" } } */
> +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
> +
> +/* Binutils PR gas/33562 */
> +/* { dg-prune-output "SVE `movprfx' compatible instruction expected" } */
> +
> +#include "test_sve_acle.h"
> +
> +#pragma GCC target "+sve2+f8f32mm"
> +
> +/*
> +** svmmla_f32f8mm_tied:
> +** msr fpmr, x0
> +** fmmla z0\.s, z4\.b, z5\.b
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f32f8mm_tied, svfloat32_t, svmfloat8_t,
> + z0 = svmmla_f32_mf8_fpm (z0, z4, z5, fpm0),
> + z0 = svmmla_fpm (z0, z4, z5, fpm0))
> +
> +/*
> +** svmmla_f32f8mm:
> +** msr fpmr, x0
> +** movprfx z0, z1
> +** fmmla z0\.s, z4\.b, z5\.b
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f32f8mm, svfloat32_t, svmfloat8_t,
> + z0 = svmmla_f32_mf8_fpm (z1, z4, z5, fpm0),
> + z0 = svmmla_fpm (z1, z4, z5, fpm0))
> +
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_sve_f16f32mm.c
> b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_sve_f16f32mm.c
> new file mode 100644
> index 00000000000..bcc86decc97
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/fmmla_sve_f16f32mm.c
> @@ -0,0 +1,31 @@
> +/* { dg-do assemble { target aarch64_asm_f16f32mm-sve_ok } } */
> +/* { dg-do compile { target { ! aarch64_asm_f16f32mm-sve_ok } } } */
> +/* { dg-skip-if "" { *-*-* } { "-DSTREAMING_COMPATIBLE" } { "" } } */
> +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
> +
> +/* Binutils PR gas/33562 */
> +/* { dg-prune-output "SVE `movprfx' compatible instruction expected" } */
> +
> +#include "test_sve_acle.h"
> +
> +#pragma GCC target "+sve-f16f32mm"
> +
> +/*
> +** svmmla_f32f16mm_tied:
> +** fmmla z0\.s, z4\.h, z5\.h
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f32f16mm_tied, svfloat32_t, svfloat16_t,
> + z0 = svmmla_f32_f16 (z0, z4, z5),
> + z0 = svmmla (z0, z4, z5))
> +
> +/*
> +** svmmla_f32f16mm:
> +** movprfx z0, z1
> +** fmmla z0\.s, z4\.h, z5\.h
> +** ret
> +*/
> +TEST_DUAL_Z (svmmla_f32f16mm, svfloat32_t, svfloat16_t,
> + z0 = svmmla_f32_f16 (z1, z4, z5),
> + z0 = svmmla (z1, z4, z5))
> +
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index 2d45ddaadc0..fa3adf94e3e 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -12535,7 +12535,7 @@ proc
> check_effective_target_aarch64_gas_has_build_attributes { } {
> set exts {
> "bf16" "cmpbr" "crc" "crypto" "dotprod" "f32mm" "f64mm" "faminmax"
> "fp" "fp8" "fp8dot2" "fp8dot4" "fp8fma" "i8mm" "ls64" "lse" "lut"
> - "sb" "simd" "sve-b16b16" "sve" "sve2"
> + "sb" "simd" "sve-b16b16" "sve" "sve2" "f8f16mm" "f8f32mm" "f16f32mm-sve"
The name of the third new extension here is "sve-f16f32mm", so let's
update this and the test immediately above which depends on it.
Hope that's all constructive, and thanks for working on this.
Alex
> }
>
> # We don't support SME without SVE2, so we'll use armv9 as the base
> --
> 2.34.1
>