Re: [PATCH] PR gas/16489: GAS doesn't complain about same register used for destination and index in AVX512 gather insn

[Michael Zolotukhin <michael dot v dot zolotukhin at gmail dot com>]

Hi HJ,

Thanks for the review, please find my answers below.

> You removed assert on operands 0 and 2, added
> check on operand 1.  Do your testcases cover
> the removal of assert on operands 0 and 2?

I didn't remove the assert on operands 0 and 2, I simply corrected it.
 This assert is for AVX2 gathers, where we have mask, destination and
index all in xmm/ymm registers.  Zmm isn't allowed there and was
previously added to this assert by mistake.  This case has been
covered by existing tests.
Added check for operand 1 covers AVX512F gathers, where we have index
and destination register, which could be in ymm or zmm register (xmm
isn't allowed there).

>
>>           if (operand_check == check_none)
>>             return 0;
>>           if (register_number (i.op[0].regs)
>> @@ -4395,6 +4393,22 @@ check_VecOperands (const insn_template *t)
>>             }
>>           as_warn (_("mask, index, and destination registers should be distinct"));
>>         }
>> +      else if (i.reg_operands == 1 && i.mask)
>> +       {
>> +         if ((i.types[1].bitfield.regymm
>> +              || i.types[1].bitfield.regzmm)
>
> Will operand 1 ever not be YMM/ZMM registers?

Yes, that takes place in scatters. For them we don't want to perform
the following checks:

>> +             && (register_number (i.op[1].regs)
>> +                 == register_number (i.index_reg)))
>> +           {
>> +             if (operand_check == check_error)
>> +               {
>> +                 i.error = invalid_vector_register_set;
>> +                 return 1;
>> +               }
>> +             if (operand_check != check_none)
>> +               as_warn (_("index and destination registers should be distinct"));
>> +           }
>> +       }

Thanks,
Michael
> H.J.