Hi Kyrill,
Sorry for the slow reply, I was away on holiday until yesterday.
On 15/07/2025 13:08, Kyrylo Tkachov wrote:
> Hi Alex,
>
> > On 15 Jul 2025, at 14:59, Alex Coplan <alex.cop...@arm.com> wrote:
> >
> > Hi,
> >
> > The predication of the SVE2 FP8 dot product insns was relying on the
> > architectural dependency:
> >
> > FEAT_FP8DOT2 => FEAT_FP8DOT4
> >
> > which was relaxed in GCC as of
> > r15-7480-g299a8e2dc667e795991bc439d2cad5ea5bd379e2, thus leading to
> > unrecognisable insn ICEs when compiling a two-way FDOT with just
> > +fp8dot2. This patch fixes the predication of the insns to test for
> > the correct feature bit depending on the value of the mode iterator.
> >
> > Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk and backport
> > to GCC 15?
> >
> > Thanks,
> > Alex
> >
> > gcc/ChangeLog:
> >
> > PR target/120986
> > * config/aarch64/aarch64-sve2.md (@aarch64_sve_dot<mode>):
> > Adjust insn predicate to use new mode attribute which checks
> > for the correct feature bit depending on the mode.
> > (@aarch64_sve_dot_lane<mode>): Likewise.
> > * config/aarch64/iterators.md (HAVE_FP8_DOT_INSN): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR target/120986
> > * gcc.target/aarch64/torture/pr120986-1.c: New test.
> > ---
> > gcc/config/aarch64/aarch64-sve2.md | 4 ++--
> > gcc/config/aarch64/iterators.md | 4 ++++
> > gcc/testsuite/gcc.target/aarch64/torture/pr120986-1.c | 10 ++++++++++
> > 3 files changed, 16 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/aarch64/torture/pr120986-1.c
> >
> > <0001-aarch64-Fix-predication-of-FP8-FDOT-insns-PR120986.patch>
>
> +(define_mode_attr HAVE_FP8_DOT_INSN [(VNx4SF "TARGET_SSVE_FP8DOT4")
> + (VNx8HF "TARGET_SSVE_FP8DOT2")])
> +
> +
>
> I think a more natural way would be to have a new mode iterator with the same
> contents as SVE_FULL_HSF, say SVE_FULL_HSF_FP8 but have the TARGET_*
> conditions guarding those modes.
> For example:
> (define_mode_iterator SVE_FULL_HSF_FP8 [(VNx4SF "TARGET_SSVE_FP8DOT4")
> (VNx8HF “TARGET_SSVE_FP8DOT2")])
Thanks for the suggestion. I agree this way seems more natural, for some reason
it hadn't occurred to me at the time to restrict the availability of the modes
themselves.
Re: naming, would you be OK with SVE_FULL_HSF_FP8_FDOT for this
iterator? Normally it's a good idea to keep iterator names as general
as possible, but the conditions on the modes make this iterator quite
specific to the FP8 FDOT instructions. Just wanted to check before pushing.
Thanks,
Alex
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/torture/pr120986-1.c
> b/gcc/testsuite/gcc.target/aarch64/torture/pr120986-1.c
> new file mode 100644
> index 00000000000..8777f1b7711
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/torture/pr120986-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=armv8.2-a+sve2+fp8dot2" } */
>
> Very minor but this bug is about arch gating rather than something that’s hit
> with optimization, I don’t think there’s value in putting it in the torture
> directory. I’d put it under normal gcc.target/aarch64.
>
> Ok with these changes if you agree.
> Thanks,
> Kyrill
>
