Re: [PATCH v3 8/9] Support APX NDD optimized encoding.

[Jan Beulich <jbeulich@suse.com>]

On 13.12.2023 07:06, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, December 12, 2023 4:46 PM
>> To: Hu, Lin1 <lin1.hu@intel.com>
>> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org; Cui, Lili
>> <lili.cui@intel.com>
>> Subject: Re: [PATCH v3 8/9] Support APX NDD optimized encoding.
>>
>> On 12.12.2023 04:18, Hu, Lin1 wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Monday, December 11, 2023 8:28 PM
>>>>
>>>> On 24.11.2023 08:02, Cui, Lili wrote:
>>>>> --- a/gas/config/tc-i386.c
>>>>> +++ b/gas/config/tc-i386.c
>>>>> @@ -7148,6 +7148,58 @@ check_APX_operands (const insn_template *t)
>>>>>    return 0;
>>>>>  }
>>>>>
>>>>> +/* Check if the instruction use the REX registers.  */ static bool
>>>>> +check_RexOperands () {
>>>>> +  for (unsigned int op = 0; op < i.operands; op++)
>>>>> +    {
>>>>> +      if (i.types[op].bitfield.class != Reg)
>>>>> +	continue;
>>>>> +
>>>>> +      if (i.op[op].regs->reg_flags & (RegRex | RegRex64))
>>>>> +	return true;
>>>>> +    }
>>>>> +
>>>>> +  if ((i.index_reg && (i.index_reg->reg_flags & (RegRex | RegRex64)))
>>>>> +      || (i.base_reg && (i.base_reg->reg_flags & (RegRex | RegRex64))))
>>>>> +    return true;
>>>>> +
>>>>> +  /* Check pseudo prefix {rex} are valid.  */  return
>>>>> + i.rex_encoding;
>>>>
>>>> Can this actually happen, when we're converting from EVEX to legacy?
>>>> (Initially I wanted to ask about "rex" and alike prefixes, i.e. the
>>>> non- pseudo
>>>> ones.)
>>>>
>>>
>>> This is to align with check_EgprOperands. I hope the function be more general.
>> Not just for this optimization problem.
>>
>> But then the comment shouldn't say "REX registers", and "Operands" in its name
>> isn't quite right either.
>>
>> Also you want to make the function be a proper modern declaration, by adding
>> "void" between the parentheses.
>>
> 
> I have modified the comment like " Check if the instruction use the REX registers or REX prefix." And function name is check_RexOperands_or_RexPrefix.

How about check_Rex_required() or is_rex_required() or some such? No need
to have two "Rex" in a single name.

Jan