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

[Cui, Lili]

> > Based on the V1, there are mainly the following changes:
> > 1. Added more test cases to cover each ins template.
> > 2. The Intel format test has been removed in this patch.
> > 3. Droped the newly added %XE and use evex_from_legacy for unified
> judgment.
> > 4. Add %ME to movbe to print {evex} correctly.
> 
> Hmm. Iirc I had outlined how to deal with this without introducing a single-use
> macro. I'm not outright opposed, but I'd like to understand why you've chosen
> not to deal with this by having decode go through mod_table[].
> 

As you predicted, with mod_table[] , we add %XE to the reg and no %XE for the memory part, but with the current implementation we want to make the judgment uniform for all map4 instructions. In this way, we cannot distinguish the reg and memory parts of movbe, because they are both in map4, so I want to add a new micro to movbe. We can also use the fixup method, but it feels not as smooth as the current one.

> > --- /dev/null
> > +++ b/gas/testsuite/gas/i386/noreg64-evex.s
> > @@ -0,0 +1,74 @@
> > +# Check 64-bit insns not sizeable through register operands with evex
> > +	{evex} sbb	$1, (%rax)
> > +	{evex} sbb	$0x89, (%rax)
> > +	{evex} sbb	$0x1234, (%rax)
> > +	{evex} sbb	$0x12345678, (%rax)
> > +	{evex} sal	$1, (%rax)
> > +	{evex} sal	$2, (%rax)
> > +	{evex} sal	%cl, (%rax)
> > +	{evex} sal	(%rax)
> > +	{evex} sar	$1, (%rax)
> > +	{evex} sar	$2, (%rax)
> > +	{evex} sar	%cl, (%rax)
> > +	{evex} sar	(%rax)
> 
> I realize it was my mistake originally, but may I ask that we don't further spread
> it: sbb really wants to come after sal and sar.
> 

Done.

> > --- 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
-------------------------------------------------------------------------------------

> >  [ 	]*[a-f0-9]+:[ 	]+08 8f c0 ff ff ff[ 	]+or.*
> >  [ 	]*[a-f0-9]+:[ 	]+62 74 7c 18 8f c0[ 	]+pop2   %rax,\(bad\)
> >  [ 	]*[a-f0-9]+:[ 	]+62 d4 24 18 8f[ 	]+\(bad\)
> >  [ 	]*[a-f0-9]+:[ 	]+c3[ 	]+.*
> >  [ 	]*[a-f0-9]+:[ 	]+62 e4 7e 08 dc 20[ 	]+aesenc128kl
> \(%rax\),%xmm20\(bad\)
> > -[ 	]*[a-f0-9]+:[ 	]+62 b4 7c 08 d9 c4[
> 	]+sha1msg1 %xmm20\(bad\),%xmm0
> > +[ 	]*[a-f0-9]+:[ 	]+62 b4 7c 08 d9 c4[ 	]+{evex}
> sha1msg1 %xmm20\(bad\),%xmm0
> 
> Why would {evex} need to appear here? There's no non-EVEX encoding
> using %xmm20, is there? It shouldn't matter that %xmm20 really is wrong to
> use here in the first place. It's (wrong) use cannot be expressed using REX2.
> 
> If having the pseudo-prefix appear here meaningfully simplifies the code, then
> at the very least the expectations here should only permit, but not demand its
> presence.
> 
> That said, I don't see how this test would have succeeded in your
> testing: There are backslashes missing to escape the figure braces.
> 

Oh, in this case xmm20 is represented by X4, and ins->rex2 doesn't have this bit, so it has to add {evex} (I forgot to add '\' for '{}', but the test case passed, which is a bit strange). Since APX spec has been updated, KEYLOCKER and SHA have been removed. We don't need to modify the code to handle X4.

> > @@ -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?

> > @@ -10547,6 +10558,11 @@ putop (instr_info *ins, const char
> *in_template, int sizeflag)
> >  		  *ins->obufp++ = '}';
> >  		  *ins->obufp++ = ' ';
> >  		  break;
> > +		case 'M':
> > +		  if (ins->modrm.mod != 3 && !(ins->rex2 & 7))
> 
> Hmm, according to the description of %ME you ought to also check for no NDD,
> even if right now the only use site (MOVBE) doesn't allow for that.
> 

Ok.

> > @@ -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:".

Thanks,
Lili.