> Hi Ezra,
Hi!
> Not an official reviewer, but some comments below:
Thanks a lot for taking a look. I just have one question.
> > --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> > @@ -2352,7 +2352,13 @@ public:
> > else
> > {
> > if (e.type_suffix_ids[1] == NUM_TYPE_SUFFIXES)
> > - icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0));
> > + {
> > + if (e.type_suffix (0).element_bytes == 2)
> > + icode = code_for_aarch64_sve2_fmmla (e.vector_mode (0),
> > + e.vector_mode (0));
> > + else
> > + icode = code_for_aarch64_sve (UNSPEC_FMMLA, e.vector_mode (0));
> > + }
>
> I think the current implementation of svmmla_impl::expand () should
> already be able to handle this, as long as your RTL patterns are defined
> ? The call to
>
> icode = code_for_aarch64_sve2 (UNSPEC_FMMLA,
> e.vector_mode (0),
> e.vector_mode (1));
>
> requires that the pattern name uses one iterator for the unspec, followed
> by two for operand modes? Something like ...
>
> ... "@aarch64_sve2_<sve_fp_op><mode><mode>" ?
>
> > + [(set (match_operand:SVE_FULL_HF_FMMLA 0 "register_operand")
> > + (unspec:SVE_FULL_HF_FMMLA
> > + [(match_operand:SVE_FULL_HF_FMMLA 2 "register_operand")
> > + (match_operand:SVE_FULL_HF_FMMLA 3 "register_operand")
> > + (match_operand:SVE_FULL_HF_FMMLA 1 "register_operand")]
> > + FMMLA))]
> > + "TARGET_NON_STREAMING && (<is_bf16> ? TARGET_SVE_B16MM : TARGET_SVE2p2)"
> > + {@ [ cons: =0 , 1 , 2 , 3 ]
> > + [ w , 0 , w , w ] <b>fmmla\t%0.h, %2.h, %3.h
> > + }
> > + [(set_attr "sve_type" "sve_fp_mul")]
> > +)
With this part, could I just have the mode appear once like this:
(define_insn "@aarch64_sve_<sve_fp_op><mode>"
since it's non-widening? Something similar appears in aarch64-sve.md:
(define_insn "@aarch64_sve_<sve_fp_op><mode>"
[(set (match_operand:SVE_MATMULF 0 "register_operand")
(unspec:SVE_MATMULF
...
so I don't think there's some convention to have it twice? I'll put it in my
next revision.
Thanks again for the reviews!
Ezra
