[PATCH v3] Support Intel AVX10.2 media instructions

[Jan Beulich]

On 08.10.2024 08:20, Jiang, Haochen wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, October 8, 2024 2:09 PM
>>
>> On 08.10.2024 04:38, Jiang, Haochen wrote:
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Monday, September 30, 2024 2:15 PM
>>>>
>>>> On 28.09.2024 17:12, Jiang, Haochen wrote:
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: Wednesday, September 25, 2024 10:42 PM
>>>>>>
>>>>>> On 25.09.2024 16:39, Haochen Jiang wrote:
>>>>>>> --- a/opcodes/i386-opc.tbl
>>>>>>> +++ b/opcodes/i386-opc.tbl
>>>>>>> @@ -3057,11 +3057,15 @@ vpdpwssd<vnni><sat>,
>> 0x6652|<sat:opc>,
>>>>>> <vnni:avx>_VNNI, Modrm|Space0F38|Src1VVVV
>>>>>>>
>>>>>>>  // {AVX512,AVX}_VNNI instructions end
>>>>>>>
>>>>>>> +<vnni_vex:int8:int16:attr:reg:mem,
>> $y:_VNNI_INT8:_VNNI_INT16:Vex::,
>>>>>> $z:10_2:10_2:Masking|Broadcast|Disp8ShiftVL:RegZMM:Dword>
>>>>>>
>>>>>> If this really is needed (contrary to previously outlined plans), it
>>>>>> wants wrapping across 3 lines (just like is done for other long-ish
>>>>>> templates). However, despite you saying something about this in the
>>>>>> description, I don't follow why this is needed.
>>>>>>
>>>>>
>>>>> The problem here is for <vnni> template, which is used for
>>>>> AVX512_VNNI/AVX_VNNI, $z comes first due to AVX512_VNNI is the
>>>>> ISA introduced earlier and we need to place it before the VEX version
>>>>> for table order.
>>>>>
>>>>> But for AVX_VNNI_INT8/16 and AVX10.2, VEX version comes first.
>>>>> We need $y placed before $z for table order.
>>>>
>>>> Well, yes, with that <vnni> can't be re-used. That's why the purging of
>>>> <vnni> is placed the way it is, ahead of the AVX-VNNI-INT{8,16} insns,
>>>> which are the ones you mean to extend. You'll want a fresh new <vnni>,
>>>> with - as you say - VEX ahead of EVEX. But yes, I clearly partially
>>>> misguided you by saying to leave alone lines that are already there; I'm
>>>> sorry for that. The two vpdpb<dpb>d<sat> and vpdpw<dpw>d<sat> lines
>> will
>>>> need touching, to add use of said 2nd instance of <vnni>. E.g.
>>>>
>>>> vpdpb<dpb>d<vnni><sat>, 0x<dpb:pfx>50|<sat:opc>, AVX<vnni:cpu>,
>>>>
>> Modrm|Space0F38|Src1VVVV|VexW0|<vnni:attr>|CheckOperandSize|NoSu
>>>> f, { RegXMM|RegYMM|<vnni:reg>|<vnni:mem>|Unspecified|BaseIndex,
>>>> RegXMM|RegYMM|<vnni:reg>, RegXMM|RegYMM|<vnni:reg> }
>>>
>>> Sorry for the late reply since I am confusing during my holiday when reading
>> the
>>> example w/o quick codebase access. However, I am even more confusing
>> today
>>> when I take a look into that.
>>>
>>> In <vnni> template, all the EVEX stuffs comes first, I suppose it could not be
>> solved
>>> only adding a <vnni:cpu> or I misunderstood it. The example will still put all
>> the evex
>>> attributes first.
>>
>> Of course. May I emphasize what I said about <vnni> in my earlier reply:
>> "That's why the purging of <vnni> is placed the way it is, ..." and "You'll
>> want a fresh new <vnni>, with - as you say - VEX ahead of EVEX". Without
>> explicit questions raised I'm in trouble seeing how this can be
>> misunderstood.
> 
> Let's get it clear.
> 
> Currently, from my understanding in the discussion, the current code change
> (adding a new <vnni_vex>) in assembler is correct and should be okayish to
> commit. Is that right?

Approximately yes. One of the issues there is the too long name. It should
be just <vnni>, without any _vex prefix (which is actually misleading anyway,
as this is only putting VEX first, but still also expands to EVEX forms).

I'd also prefer if you didn't move around the purging of <dpb> and <dpw>.

> I am confused by the example actually, it is using the old
> <vnni>. If that is the case, I have no problem with that.

Why "old"? I can only keep saying that the purging of the old <vnni> sits
where it sits for a reason, and that you want to introduce a 2nd, fresh <vnni>
ahead of the new templates you add. Apparently all you need to do is rename
vnni_vex to just vnni.

Jan