[PATCH V2] Support {evex} pseudo prefix for decode evex promoted insns without egpr32.

[Jan Beulich]

On 02.04.2024 10:59, Cui, Lili wrote:
>>> --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d
>>> @@ -30,16 +30,16 @@ Disassembly of section .text:
>>>  [ 	]*[a-f0-9]+:[ 	]+0c 18[ 	]+or.*
>>>  [ 	]*[a-f0-9]+:[ 	]+62 f2 fc 18 f5[ 	]+\(bad\)
>>>  [ 	]*[a-f0-9]+:[ 	]+0c 18[ 	]+or.*
>>> -[ 	]*[a-f0-9]+:[ 	]+62 f4 e4[ 	]+\(bad\)
>>> +[ 	]*[a-f0-9]+:[ 	]+62 f4 e4[ 	]+\{evex\} \(bad\)
>>>  [ 	]*[a-f0-9]+:[ 	]+08 ff[ 	]+.*
>>>  [ 	]*[a-f0-9]+:[ 	]+04 08[ 	]+.*
>>> -[ 	]*[a-f0-9]+:[ 	]+62 f4 3c[ 	]+\(bad\)
>>> +[ 	]*[a-f0-9]+:[ 	]+62 f4 3c[ 	]+\{evex\} \(bad\)
>>
>> Why is this? What's the criteria for {evex} to appear ahead of (bad)? And if so
>> for EVEX, shouldn't VEX gain {vex} in such cases, too? (Which is really the
>> opposite I mean to indicate: No such prefixes should ever appear here. If
>> anything we should present unrecognized VEX/EVEX encodings in a sufficiently
>> generic way, including all of their - similarly generalized -
>> operands.)
> 
> Our rules for adding {evex} to map4 are:
> 1. No NDD( means ins->vex.nd is 0).
> 2. No Egprs( use ins->rex2, excluding X4).
> 3. Other macros are not added {evex}/{nf}.
> 
> For {evex} inc %rax %rbx, we set ins->vex.nd = 0, meaning it only has two operands, I think it is right to add {evex} for it.
> -----------------------------------------------------------------------------------
>         #{evex} inc %rax %rbx EVEX.vvvv != 1111 && EVEX.ND = 0.
>         .byte 0x62, 0xf4, 0xe4, 0x08, 0xff, 0x04, 0x08
> -----------------------------------------------------------------------------------
> 
> For pop2 %rax,%r8, it only has EVEX format, it's special because its ins->vex.nd != 0,so the normal process will not add {evex} to it, but we give it an illegal value, let ins->vex.nd = 0, so it added {evex} by mistake. This mistake is caused by illegal values. I don’t have a reasonable fix, so I prefer not to change it.
> ------------------------------------------------------------------------------------
>         # pop2 %rax, %r8 set EVEX.ND=0.
>         .byte 0x62, 0xf4, 0x3c, 0x08, 0x8f, 0xc0
>         .byte 0xff, 0xff, 0xff
> -------------------------------------------------------------------------------------

The POP2 aspect isn't really relevant here. Imo (bad) should never be prefixed
by (pseudo) prefixes. If, however, it is to be, then such prefixing needs doing
consistently. Which I'm afraid is going to be quite a bit more work than simply
zapping (or avoiding) {evex} when (bad) is printed.

>>> @@ -10398,6 +10402,7 @@ putop (instr_info *ins, const char *in_template,
>> int sizeflag)
>>>    int cond = 1;
>>>    unsigned int l = 0, len = 0;
>>>    char last[4];
>>> +  bool b_done = false;
>>
>> Mind me asking what "b" in this identifier is intended to stand for?
>>
> 
> I just want to show that it is a bool type. Maybe it would be better to change it to b_added_evex_prefix? Do you have any suggestions?

I don't think we use such b_ prefixes elsewhere. "evex_printed" or some such
would seem sufficient.

>>> @@ -10588,7 +10604,11 @@ putop (instr_info *ins, const char
>> *in_template, int sizeflag)
>>>  		  oappend (ins, "{nf} ");
>>>  		  /* This bit needs to be cleared after it is consumed.  */
>>>  		  ins->vex.nf = false;
>>> +		  b_done = true;
>>>  		}
>>> +	      else if (ins->evex_type == evex_from_vex && !(ins->rex2 & 7)
>>> +		       && ins->vex.v)
>>> +		oappend (ins, "{evex} ");
>>
>> Why would b_done not need setting here as well?
>>
> 
> We only use b_done under "ins->evex_type == evex_from_legacy".  Original version, we also handle "evex_from_legacy" here, now it is merged directly into "default:".

Then leaving a trap for later? Let's keep internal state properly updated:
If {evex} is printed, reflect that in local variable state, too.

Jan