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

[Jan Beulich]

(sorry, previous mail went out unfinished by mistake)

On 22.03.2024 10:49, Cui, Lili wrote:
> From: "Hu, Lin1" <lin1.hu@intel.com>
> 
> 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[].

> --- /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
> +
> +        .text
> +_start:
> +	{evex} adc	$1, (%rax)
> +	{evex} adc	$0x89, (%rax)
> +	{evex} adc	$0x1234, (%rax)
> +	{evex} adc	$0x12345678, (%rax)
> +	{evex} add	$1, (%rax)
> +	{evex} add	$0x89, (%rax)
> +	{evex} add	$0x1234, (%rax)
> +	{evex} add	$0x12345678, (%rax)
> +	{evex} and	$1, (%rax)
> +	{evex} and	$0x89, (%rax)
> +	{evex} and	$0x1234, (%rax)
> +	{evex} and	$0x12345678, (%rax)
> +	{evex} crc32	(%rax), %eax
> +	{evex} crc32	(%rax), %rax
> +	{evex} dec	(%rax)
> +	{evex} div	(%rax)
> +	{evex} idiv	(%rax)
> +	{evex} imul	(%rax)
> +	{evex} inc	(%rax)
> +	{evex} mul	(%rax)
> +	{evex} neg	(%rax)
> +	{evex} not	(%rax)
> +	{evex} or 	$1, (%rax)
> +	{evex} or 	$0x89, (%rax)
> +	{evex} or 	$0x1234, (%rax)
> +	{evex} or 	$0x12345678, (%rax)
> +	{evex} rcl	$1, (%rax)
> +	{evex} rcl	$2, (%rax)
> +	{evex} rcl	%cl, (%rax)
> +	{evex} rcl	(%rax)
> +	{evex} rcr	$1, (%rax)
> +	{evex} rcr	$2, (%rax)
> +	{evex} rcr	%cl, (%rax)
> +	{evex} rcr	(%rax)
> +	{evex} rol	$1, (%rax)
> +	{evex} rol	$2, (%rax)
> +	{evex} rol	%cl, (%rax)
> +	{evex} rol	(%rax)
> +	{evex} ror	$1, (%rax)
> +	{evex} ror	$2, (%rax)
> +	{evex} ror	%cl, (%rax)
> +	{evex} ror	(%rax)
> +	{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.

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

>  [ 	]*[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.

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

> @@ -10411,6 +10416,12 @@ putop (instr_info *ins, const char *in_template, int sizeflag)
>        switch (*p)
>  	{
>  	default:
> +	  if (ins->evex_type == evex_from_legacy && !ins->vex.nd
> +	      && !(ins->rex2 & 7) && !b_done)
> +	    {
> +	      oappend (ins, "{evex} ");
> +	      b_done = true;
> +	    }
>  	  *ins->obufp++ = *p;
>  	  break;

Right now it looks like it is okay to do this here; let's hope this
isn't going to bite us later.

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

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

Jan