Re: [PATCH v2 1/4] x86: zap value-less Disp8MemShift from non-EVEX templates

[Jan Beulich <jbeulich@suse.com>]

On 24.04.2024 15:15, Cui, Lili wrote:
>> On 24.04.2024 08:49, Cui, Lili wrote:
>>>> In order to allow to continue to use templatized SSE2AVX templates
>>>> when enhancing those to also cover eGPR usage, Disp8MemShift wants
>>>> using to deviate from what general template attributes supply. That
>>>> requires using Disp8MemShift in a way also affecting non-EVEX
>>>> templates, yet having this attribute set would so far implicitly mean EVEX
>> encoding.
>>>> Recognize the case and instead zap the attribute if no other
>>>> attribute indicates EVEX encoding.
>>>>
>>>
>>> I'm confused about this patch, is it related to the movsd template? You
>> removed the "Masking" for it and only left Disp8MemShift, but I thought it
>> still belongs to EVEX template.
>>>
>>> +movsd, 0xf210, AVX512F,
>>> +D|Modrm|EVexLIG|Space0F|VexW1|Disp8MemShift=3|NoSuf|SSE2AVX,
>> {
>>> +Qword|Unspecified|BaseIndex, RegXMM }
>>
>> There's no "masking" in an SSE2AVX template. Use of masking in a legacy-
>> form input instruction is simply wrong, and wants rejecting.
>>
> 
> Is this patch intended to fix the following situation?

Yes, this is one of the cases where the change particularly matters.

> If not, could you give an example?
> 
> <sse41:cpu:attr:scal:vvvv, $avx:AVX:Vex128|VexW0|SSE2AVX:VexLIG|VexW0|SSE2AVX:VexVVVV, $sse:SSE4_1:::>
> 
> +<SSE41D:cpu:attr:scal:vvvv, +
> +    $avx:AVX|AVX512VL:Vex128|EVex128|VexW0|Disp8MemShift=4|SSE2AVX:VexLIG|EVexLIG|VexW0|Disp8MemShift=2|SSE2AVX:VexVVVV, +
> +    $sse:SSE4_1:::>
> 
> -insertps<sse41>, 0x660f3a21, <sse41:cpu>, Modrm|<sse41:attr>|<sse41:vvvv>|NoSuf, { Imm8, Dword|Unspecified|BaseIndex|RegXMM, RegXMM }
> +insertps<SSE41D>, 0x660f3a21, <SSE41D:cpu>, Modrm|<SSE41D:attr>|<SSE41D:vvvv>|Disp8MemShift|NoSuf, { Imm8, Dword|Unspecified|BaseIndex|RegXMM, RegXMM }
> 
> I'm confused why the Disp8MemShift is added here.

<SSE41D:attr> includes Disp8MemShift=4, which would be wrong here. And
<SSE41D:scal> also cannot be used, for specifying VexLIG|EVexLIG. Hence
Disp8MemShift without a value is used to override the earlier setting
(coming from <SSE41D:attr>). Yet by having it here, the legacy (non-
SSE2AVX) template would also have that attribute, which would be wrong
(we'd infer EVEX encoding from its presence). Hence why such specific
instances need zapping in i386-gen.

Jan