Re: [PATCH 2/8] Support APX GPR32 with extend evex prefix

[Jan Beulich <jbeulich@suse.com>]

On 19.10.2023 18:38, Cui, Lili wrote:
>>> @@ -9063,6 +9066,22 @@ get_valid_dis386 (const struct dis386 *dp,
>> instr_info *ins)
>>>         }
>>>
>>>        ins->need_vex = 4;
>>> +
>>> +      /* EVEX from legacy requrie EVEX.P[17:16] must be 0, EVEX.P[23:21]
>> must
>>> +        be 0.
>>> +        EVEX from evex requrie EVEX.P[17:16] must be 0. EVEX.P[23:22] must
>>> +        be 0, EVEX.P[20] must be 0.  */
>>> +      if (ins->evex_type == evex_from_legacy || ins->evex_type ==
>> evex_from_vex)
>>> +       {
>>> +         if (!((*ins->codep & 0x3) == 0)
>>> +             || !((*ins->codep >> 6 & 0x3) == 0)
>>> +             || (ins->evex_type == evex_from_legacy
>>> +                 && !((*ins->codep >> 5 & 0x1) == 0))
>>> +             || (ins->evex_type == evex_from_vex
>>> +                 && !ins->vex.b))
>>> +           return &bad_opcode;
>>
>> I guess I'm confused here: So far we don't use EVEX.P[] as notation in
>> comments. Can you please use the respective field names instead? Also can
>> you please improve readability by converting !(a == b) into a != b?
>>
> 
> Changed it to:
> 
> +      /* EVEX from legacy instructions requrie vex.mask_register_specifier, vex.ll
> +        and vex.zeroing must be 0.
> +        EVEX from evex instrucions requrie vex.mask_register_specifier and vex.ll
> +        must be 0.  */
> +      if (ins->evex_type == evex_from_legacy || ins->evex_type == evex_from_vex)
> +       {
> +         if ((*ins->codep & 0x3) != 0
> +             || (*ins->codep >> 6 & 0x3) != 0
> +             || (ins->evex_type == evex_from_legacy
> +                 && (*ins->codep >> 5 & 0x1) != 0)
> +             || (ins->evex_type == evex_from_vex
> +                 && !ins->vex.b))
> +           return &bad_opcode;
> +       }

I guess my earlier response was ambiguous, I'm sorry: I didn't mean the
disassembler's internal names, but the field names as per the SDM (e.g.
EVEX.W). Also (nit) please avoid typo-ing "require" even twice.

Jan