[PATCH] x86: Determine vector length from the last vector operand

[H.J. Lu]

On Tue, Jul 24, 2018 at 12:41 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 22.07.18 at 21:02, <hjl.tools@gmail.com> wrote:
>> Determine VEX/EVEXE vector length from the last multi-length vector
>> operand.
>
> I see you've committed this already: It would have been really
> helpful to say _why_ you're doing the change in the commit message.
> For posterity as well as my understanding - could you at least do so
> here please? That's even more so that VEX and EVEX processing
> differed in that regard before your change.

The encoding can be determined by the last  multi-length vector
operand.  There is no need to scan from the start.

>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -3363,10 +3363,12 @@ build_vex_prefix (const insn_template *t)
>>      vector_length = 1;
>>    else
>>      {
>> -      unsigned int op;
>> +      int op;
>
> This is the sort of change I would have objected to, btw. Variables
> used to index arrays should be unsigned whenever possible. And
> doing so would have been easy enough here:

I want

if (op < 0)
  abort ();

after the loop.

>> @@ -3611,20 +3613,31 @@ build_evex_prefix (void)
>>        if (!i.tm.opcode_modifier.evex
>>         || i.tm.opcode_modifier.evex == EVEXDYN)
>>       {
>> -       unsigned int op;
>> +       int op;
>>
>> +       /* Determine vector length from the last multi-length vector
>> +          operand.  */
>>         vec_length = 0;
>> -       for (op = 0; op < i.tm.operands; ++op)
>> +       for (op = i.operands - 1; op >= 0; op--)
>
>           for (op = i.operands; op--; )
>
> (similarly further down).

They are equivalent.

>>           if (i.tm.operand_types[op].bitfield.xmmword
>>               + i.tm.operand_types[op].bitfield.ymmword
>>               + i.tm.operand_types[op].bitfield.zmmword > 1)
>>             {
>>               if (i.types[op].bitfield.zmmword)
>> -               i.tm.opcode_modifier.evex = EVEX512;
>> +               {
>> +                 i.tm.opcode_modifier.evex = EVEX512;
>> +                 break;
>> +               }
>>               else if (i.types[op].bitfield.ymmword)
>> -               i.tm.opcode_modifier.evex = EVEX256;
>> +               {
>> +                 i.tm.opcode_modifier.evex = EVEX256;
>> +                 break;
>> +               }
>>               else if (i.types[op].bitfield.xmmword)
>> -               i.tm.opcode_modifier.evex = EVEX128;
>> +               {
>> +                 i.tm.opcode_modifier.evex = EVEX128;
>> +                 break;
>> +               }
>>               else if (i.broadcast && (int) op == i.broadcast->operand)
>
> This has rendered all the "else" pointless.

What do you mean? Isn't it used in

vcvttpd2uqq, 2, 0x6678, None, 1, CpuAVX512DQ,
Modrm|Masking=3|VexOpcode=0|VexW=2|Broadcast|Disp8ShiftVL|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,
{ RegXMM|RegYMM|RegZMM|Qword|Unspecified|BaseIndex,
RegXMM|RegYMM|RegZMM }


-- 
H.J.