[PATCH 2/3] x86: Drop SwapSources

Jan Beulich jbeulich@suse.com
Fri Apr 26 10:37:00 GMT 2024


On 26.04.2024 10:14, Cui, Lili wrote:
>> On 24.04.2024 09:23, Cui, Lili wrote:
>>> --- a/gas/config/tc-i386.c
>>> +++ b/gas/config/tc-i386.c
>>> @@ -10434,6 +10434,14 @@ build_modrm_byte (void)
>>>
>>>    switch (i.tm.opcode_modifier.vexvvvv)
>>>      {
>>> +    case VexVVVV_SRC2:
>>> +      if (source != op)
>>> +	{
>>> +	  v = source++;
>>> +	  break;
>>> +	}
>>> +      /* For XOP: vpshl* and vpsha*.  */
>>> +      /* Fall through.  */
>>>      case VexVVVV_SRC1:
>>
>> This falling-through is odd and hence needs a better comment (then also
>> covering vprot*, which afaict is similarly affected). The reason for this is the
>> XOP.W-controlled operand swapping, if I'm not mistaken? In which case
>> perhaps instead of the fall-through here the logic swapping the operands
>> should replace VexVVVV_SRC2 by VexVVVV_SRC1?
>>
> 
> Yes, vprot* should be included, and it is related to XOP.W-controlled operand swapping, the comments says " /* Only the first two register operands need reversing, alongside flipping VEX.W.  */ ",  But there is actually a memory operand, not two register operands.
> 
> I think VexVVVV_SRC2 makes more sense here, it matches the actual situation, we want to use vvvv to encode the first operand.
> 
> Opcode table:
> vprot<xop>, 0x90 | <xop:opc>, XOP, D|Modrm|Vex128|SpaceXOP09|VexVVVV_Src2|VexW0|NoSuf, { RegXMM, RegXMM|Unspecified|BaseIndex, RegXMM }
> 
> testcase:
> vprotb (%rax),%xmm12,%xmm15
> vprotb %xmm15,(%r12),%xmm0

VexVVVV_Src2 is appropriate for the latter, yes, but not for the former. That
uses VexVVVV_Src1 layout. Hence my suggestion to replace the attribute when
swapping operands.

Jan


More information about the Binutils mailing list