> On 5 Aug 2025, at 11:00, Alex Coplan <alex.cop...@arm.com> wrote:
>
> 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.
>
Yes that’s fine, I don’t feel strongly about it.
Thanks,
Kyrill
> 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
