public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Cui, Lili" <lili.cui@intel.com>
Cc: hongjiu.lu@intel.com, binutils@sourceware.org
Subject: Re: [PATCH 7/8] Support APX NF
Date: Thu, 28 Sep 2023 14:42:26 +0200	[thread overview]
Message-ID: <99f05102-859c-5882-07f3-4c2da54c0e80@suse.com> (raw)
In-Reply-To: <20230919152527.497773-8-lili.cui@intel.com>

On 19.09.2023 17:25, Cui, Lili wrote:
> @@ -4178,11 +4180,15 @@ build_evex_insns_with_extend_evex_prefix (void)
>      i.vex.bytes[1] &= 0xef;
>    if (i.vex.register_specifier
>        && register_number (i.vex.register_specifier) > 0xf)
> -    i.vex.bytes[3] &=0xf7;
> +    i.vex.bytes[3] &= 0xf7;

When you notice such issues, they want correcting in the patch introducing
them.

> @@ -5944,6 +5954,10 @@ parse_insn (const char *line, char *mnemonic, bool prefix_only)
>  		  /* {rex2} */
>  		  i.rex2_encoding = true;
>  		  break;
> +		case Prefix_NF:
> +		  /* {NF} */
> +		  i.has_nf = true;
> +		  break;

I find it odd that this is represented as a (pseudo-)prefix. The manual
doesn't suggest so; it rather looks like the intention is for it to be a
mnemonic suffix, as in "add{nf} ...". Hence same question as before: In
how far is this representation aligned with what other assemblers are
going to do?

> @@ -7151,6 +7165,19 @@ optimize_NDD_to_nonNDD (const insn_template *t)

How useful that this function is mentioned at least this way: No change
there? (See my comments on the patch introducing it.)

>    return 0;
>  }
>  
> +/* Check if NF prefix requirements are met by the instruction.  */
> +static int

As before, bool please for functions returning boolean values.

> +check_NfPrefix (const insn_template *t)
> +{
> +  if (i.has_nf && !t->opcode_modifier.nf)
> +    {
> +      /* This instruction should support nf prefix.  */
> +      i.error = unsupported;

A more specific error message would be nice here.

Question of course is whether, for such an isolated check, you really
need a new helper function.

> @@ -7551,6 +7578,7 @@ match_template (char mnem_suffix)
>  		  goto check_operands_345;
>  		}
>  	      else if (t->opcode_space != SPACE_BASE
> +		       && !t->opcode_modifier.nf
>  		       && (t->opcode_space != SPACE_0F
>  			   /* MOV to/from CR/DR/TR, as an exception, follow
>  			      the base opcode space encoding model.  */

With an earlier comment addressed, I expect this change may not be
necessary anymore.

> @@ -7652,6 +7680,13 @@ match_template (char mnem_suffix)
>  	  continue;
>  	}
>  
> +      /* Check if nf prefix are valid.  */
> +      if (check_NfPrefix (t))
> +	{
> +	  specific_error = progress (i.error);
> +	  continue;
> +	}

Is it helpful (e.g. diagnostic-wise) to have this check so late? If so,
is it useful to "continue" when this is the only thing that doesn't match?
No other template is going to match in such an event, afaict.

> --- a/gas/testsuite/gas/i386/x86-64-apx-ndd.d
> +++ b/gas/testsuite/gas/i386/x86-64-apx-ndd.d
> @@ -158,7 +158,7 @@ Disassembly of section .text:
>  \s*[a-f0-9]+:\s*67 62 f4 3c 18 4f 90 90 90 90 90 	cmovg  -0x6f6f6f70\(%eax\),%edx,%r8d
>  \s*[a-f0-9]+:\s*67 62 f4 3c 18 af 90 09 09 09 00 	imul   0x90909\(%eax\),%edx,%r8d
>  \s*[a-f0-9]+:\s*62 b4 b0 10 af 94 f8 09 09 00 00 	imul   0x909\(%rax,%r31,8\),%rdx,%r25
> -\s*[a-f0-9]+:\s*62 f4 fc 08 ff c0\s+inc    %rax
> +\s*[a-f0-9]+:\s*62 f4 fc 08 ff c0\s+\{evex\} inc %rax

It's kind of unexpected to see this change here.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-nf.s
> @@ -0,0 +1,1256 @@
> +# Check 64bit APX_F instructions
> +
> +        .text
> +_start:
> +	{nf}	add	$123, %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX

Comments on earlier patches apply throughout this file as well.

> +	{nf}	add	$123, %bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	$123, %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	$123, %dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	$123, %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	$123, %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	$123, %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	$123, %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	addb	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	addb	$123, 291(%r8, %rax, 4), %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	addw	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	addw	$123, 291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	addl	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	addl	$123, 291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	addq	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	addq	$123, 291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%bl, %dl, %r8b	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%bl, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%bl, 291(%r8, %rax, 4), %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%dx, %ax, %r9w	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%dx, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%dx, 291(%r8, %rax, 4), %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%ecx, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%ecx, 291(%r8, %rax, 4), %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	%r9, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	%r9, 291(%r8, %rax, 4), %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	291(%r8, %rax, 4), %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	291(%r8, %rax, 4), %bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	291(%r8, %rax, 4), %dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	291(%r8, %rax, 4), %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	add	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	add	291(%r8, %rax, 4), %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	$123, %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	$123, %bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	$123, %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	$123, %dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	$123, %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	$123, %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	$123, %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	$123, %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	andb	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andb	$123, 291(%r8, %rax, 4), %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	andw	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andw	$123, 291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	andl	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andl	$123, 291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	andq	$123, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andq	$123, 291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%bl, %dl, %r8b	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%bl, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%bl, 291(%r8, %rax, 4), %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%dx, %ax, %r9w	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%dx, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%dx, 291(%r8, %rax, 4), %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%ecx, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%ecx, 291(%r8, %rax, 4), %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	%r9, 291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	%r9, 291(%r8, %rax, 4), %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	291(%r8, %rax, 4), %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	291(%r8, %rax, 4), %bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	291(%r8, %rax, 4), %dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	291(%r8, %rax, 4), %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	and	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	and	291(%r8, %rax, 4), %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	andn	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andn	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andn	291(%r8, %rax, 4), %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	andn	291(%r8, %rax, 4), %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bextr	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bextr	%ecx, 291(%r8, %rax, 4), %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bextr	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bextr	%r9, 291(%r8, %rax, 4), %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsi	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsi	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsi	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsi	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsmsk	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsmsk	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsmsk	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsmsk	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsr	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsr	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsr	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	blsr	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bzhi	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bzhi	%ecx, 291(%r8, %rax, 4), %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bzhi	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	bzhi	%r9, 291(%r8, %rax, 4), %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	dec	%bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	dec	%bl, %dl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	dec	%dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	dec	%dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	dec	%ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	dec	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	dec	%r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	dec	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	decb	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	decb	291(%r8, %rax, 4), %bl	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	decw	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	decw	291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	decl	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	decl	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	decq	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	decq	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	div	%bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	div	%dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	div	%ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	div	%r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	divb	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	divw	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	divl	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	divq	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idiv	%bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idiv	%dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idiv	%ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idiv	%r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idivb	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idivw	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idivl	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	idivq	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%bl	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%dx, %ax, %r9w	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	imul	%ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%ecx, %edx, %r10d	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	imul	%r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	%r9, %r31, %r11	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	imulb	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imulw	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %dx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %dx, %ax	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	imull	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %ecx	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %ecx, %edx	 #APX_F OPC_EVEX_NF OPC_EVEX_ND
> +	{nf}	imulq	291(%r8, %rax, 4)	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %r9	 #APX_F OPC_EVEX_NF OPC_EVEX_EVEX
> +	{nf}	imul	291(%r8, %rax, 4), %r9, %r31	 #APX_F OPC_EVEX_NF OPC_EVEX_ND

No IMUL by immediate?

> --- a/gas/testsuite/gas/i386/x86-64.exp
> +++ b/gas/testsuite/gas/i386/x86-64.exp
> @@ -372,6 +372,8 @@ run_dump_test "x86-64-apx-evex-promoted"
>  run_dump_test "x86-64-apx-evex-promoted-intel"
>  run_dump_test "x86-64-apx-evex-egpr"
>  run_dump_test "x86-64-apx-ndd"
> +run_dump_test "x86-64-apx-nf"
> +run_dump_test "x86-64-apx-nf-intel"
>  run_dump_test "x86-64-avx512f-rcigrz-intel"
>  run_dump_test "x86-64-avx512f-rcigrz"
>  run_dump_test "x86-64-clwb"

No test checking that {nf} isn't accepted (assembler) / EVEX.nf set is
rejected (disassembler) on insns not permitting its use, at least for a
few examples?

> @@ -1003,7 +1007,7 @@ typedef struct insn_template
>       AMD 3DNow! instructions.
>       If this template has no extension opcode (the usual case) use None
>       Instructions */
> -  signed int extension_opcode:0xA;
> +  signed int extension_opcode:0xB;

For this and ...

> @@ -1017,7 +1021,8 @@ typedef struct insn_template
>  #define Prefix_EVEX		7	/* {evex} */
>  #define Prefix_REX		8	/* {rex} */
>  #define Prefix_REX2		9	/* {rex2} */
> -#define Prefix_NoOptimize	0xA	/* {nooptimize} */
> +#define Prefix_NF		0xA	/* {nf} */
> +#define Prefix_NoOptimize	0xB	/* {nooptimize} */

... this, see comments on an earlier patch.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -286,25 +286,41 @@ add, 0x0, 0, D|W|CheckOperandSize|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg3
>  add, 0x83/0, 0, Modrm|No_bSuf|No_sSuf|HLEPrefixLock, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex }
>  add, 0x4, 0, W|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword }
>  add, 0x80/0, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
> -add, 0x0, APX_F|x64, D|W|CheckOperandSize|Modrm|No_sSuf|VexVVVV|EVex128|EVexMap4, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -add, 0x83/0, APX_F|x64, Modrm|CheckOperandSize|No_bSuf|No_sSuf|VexVVVV|EVex128|EVexMap4, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
> -add, 0x80/0, APX_F|x64, W|Modrm|CheckOperandSize|No_sSuf|VexVVVV|EVex128|EVexMap4, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64}
> +
> +add, 0x0, APX_F|x64, D|W|CheckOperandSize|Modrm|No_sSuf|EVex128|EVexMap4|NF, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
> +add, 0x83/0, APX_F|x64, Modrm|No_bSuf|No_sSuf|EVex128|EVexMap4|NF, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex }
> +add, 0x80/0, APX_F|x64, W|Modrm|No_sSuf|EVex128|EVexMap4|NF, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }

Huge patches like this are already hard enough to review. Can you please try to
make sure you introduce new templates right in their final shape (within the
specific series of course), rather than touching them again a 2nd time? Even
without fully supporting NF, introducing the attribute (as a dummy or without
any consumer) ought to be possible earlier on.

As per earlier comments many of these templates need cleaning up anyway, so I
won't look at the other in any detail here, and instead wait for a v2.

Jan

  parent reply	other threads:[~2023-09-28 12:42 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 15:25 [PATCH 0/8] [RFC] Support Intel APX EGPR Cui, Lili
2023-09-19 15:25 ` [PATCH 1/8] Support APX GPR32 with rex2 prefix Cui, Lili
2023-09-21 15:27   ` Jan Beulich
2023-09-27 15:57     ` Cui, Lili
2023-09-21 15:51   ` Jan Beulich
2023-09-27 15:59     ` Cui, Lili
2023-09-28  8:02       ` Jan Beulich
2023-10-07  3:27         ` Cui, Lili
2023-09-19 15:25 ` [PATCH 2/8] Support APX GPR32 with extend evex prefix Cui, Lili
2023-09-22 10:12   ` Jan Beulich
2023-10-17 15:48     ` Cui, Lili
2023-10-18  6:40       ` Jan Beulich
2023-10-18 10:44         ` Cui, Lili
2023-10-18 10:50           ` Jan Beulich
2023-09-22 10:50   ` Jan Beulich
2023-10-17 15:50     ` Cui, Lili
2023-10-17 16:11       ` Jan Beulich
2023-10-18  2:02         ` Cui, Lili
2023-10-18  6:10           ` Jan Beulich
2023-09-25  6:03   ` Jan Beulich
2023-10-17 15:52     ` Cui, Lili
2023-10-17 16:12       ` Jan Beulich
2023-10-18  6:31         ` Cui, Lili
2023-10-18  6:47           ` Jan Beulich
2023-10-18  7:52             ` Cui, Lili
2023-10-18  8:21               ` Jan Beulich
2023-10-18 11:30                 ` Cui, Lili
2023-10-19 11:58                   ` Cui, Lili
2023-10-19 15:24                     ` Jan Beulich
2023-10-19 16:38                       ` Cui, Lili
2023-10-20  6:25                         ` Jan Beulich
2023-10-22 14:33                           ` Cui, Lili
2023-09-19 15:25 ` [PATCH 3/8] Add tests for " Cui, Lili
2023-09-27 13:11   ` Jan Beulich
2023-10-17 15:53     ` FW: " Cui, Lili
2023-10-17 16:19       ` Jan Beulich
2023-10-18  2:32         ` Cui, Lili
2023-10-18  6:05           ` Jan Beulich
2023-10-18  7:16             ` Cui, Lili
2023-10-18  8:05               ` Jan Beulich
2023-10-18 11:26                 ` Cui, Lili
2023-10-18 12:06                   ` Jan Beulich
2023-10-25 16:03                     ` Cui, Lili
2023-09-27 13:19   ` Jan Beulich
2023-09-19 15:25 ` [PATCH 4/8] Support APX NDD Cui, Lili
2023-09-27 14:44   ` Jan Beulich
2023-10-22 14:05     ` Cui, Lili
2023-10-23  7:12       ` Jan Beulich
2023-10-25  8:10         ` Cui, Lili
2023-10-25  8:47           ` Jan Beulich
2023-10-25 15:49             ` Cui, Lili
2023-10-25 15:59               ` Jan Beulich
2023-09-28  7:57   ` Jan Beulich
2023-10-22 14:57     ` Cui, Lili
2023-10-24 11:39     ` Cui, Lili
2023-10-24 11:58       ` Jan Beulich
2023-10-25 15:29         ` Cui, Lili
2023-09-19 15:25 ` [PATCH 5/8] Support APX NDD optimized encoding Cui, Lili
2023-09-28  9:29   ` Jan Beulich
2023-10-23  2:57     ` Hu, Lin1
2023-10-23  7:23       ` Jan Beulich
2023-10-23  7:50         ` Hu, Lin1
2023-10-23  8:15           ` Jan Beulich
2023-10-24  1:40             ` Hu, Lin1
2023-10-24  6:03               ` Jan Beulich
2023-10-24  6:08                 ` Hu, Lin1
2023-10-23  3:07     ` [PATCH-V2] " Hu, Lin1
2023-10-23  3:30     ` [PATCH 5/8] [v2] " Hu, Lin1
2023-10-23  7:26       ` Jan Beulich
2023-09-19 15:25 ` [PATCH 6/8] Support APX Push2/Pop2 Cui, Lili
2023-09-28 11:37   ` Jan Beulich
2023-10-30 15:21     ` Cui, Lili
2023-10-30 15:31       ` Jan Beulich
2023-11-20 13:05         ` Cui, Lili
2023-09-19 15:25 ` [PATCH 7/8] Support APX NF Cui, Lili
2023-09-25  6:07   ` Jan Beulich
2023-09-28 12:42   ` Jan Beulich [this message]
2023-11-02 10:15     ` Cui, Lili
2023-11-02 10:23       ` Jan Beulich
2023-11-02 10:46         ` Cui, Lili
2023-12-12  2:59           ` H.J. Lu
2023-09-19 15:25 ` [PATCH 8/8] Support APX JMPABS Cui, Lili
2023-09-28 13:11   ` Jan Beulich
2023-11-02  2:32     ` Hu, Lin1

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=99f05102-859c-5882-07f3-4c2da54c0e80@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=hongjiu.lu@intel.com \
    --cc=lili.cui@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).