[PATCH 1/3] Support Intel AMX-TRANSPOSE

[Jan Beulich]

On 18.12.2024 07:32, Haochen Jiang wrote:
> @@ -10750,25 +10752,43 @@ process_operands (void)
>        unsigned int op, extra;
>        const reg_entry *first;
>  
> -      /* The second operand must be {x,y,z}mmN. */
> -      gas_assert (i.operands == 3 && i.types[1].bitfield.class == RegSIMD);
> +      /* The second operand must be {x,y,z,t}mmN */
> +      gas_assert ((i.operands == 2 || i.operands == 3)
> +		  && i.types[1].bitfield.class == RegSIMD);
>  
> -      switch (i.types[2].bitfield.class)
> +      if (i.operands == 3)
>  	{
> -	case RegSIMD:
> -	  /* AVX512-{4FMAPS,4VNNIW} operand 2: N must be a multiple of 4. */
> -	  op = 1;
> -	  extra = 3;
> -	  break;
> +	  switch (i.types[2].bitfield.class)
> +	    {
> +	    case RegSIMD:
> +	      /* AVX512-{4FMAPS,4VNNIW} operand 2: N must be a multiple of 4. */
> +	      op = 1;
> +	      extra = 3;
> +	      break;
>  
> -	case RegMask:
> -	  /* AVX512-VP2INTERSECT operand 3: N must be a multiple of 2. */
> -	  op = 2;
> -	  extra = 1;
> -	  break;
> +	    case RegMask:
> +	      /* AVX512-VP2INTERSECT operand 3: N must be a multiple of 2. */
> +	      op = 2;
> +	      extra = 1;
> +	      break;
>  
> -	default:
> -	  abort ();
> +	    default:
> +	      abort ();
> +	    }
> +	}
> +      else
> +	{
> +	  switch (i.types[1].bitfield.class)
> +	    {
> +	    case RegSIMD:
> +	      /* AMX-TRANSPOSE operand 2: N must be a multiple of 2. */
> +	      op = 1;
> +	      extra = 1;
> +	      break;
> +
> +	    default:
> +	      abort ();
> +	    }
>  	}

This could have been done with less churn, also making it easier to review.
There's imo no need to wrap an operand count check around the switch().
Instead in the RegSIMD case you can check the register type (Tmmword), thus
likely making the new code a simple insertion. That's what I had in mind
when originally laying out the code that you're now fully re-indenting.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-amx-transpose-bad.s
> @@ -0,0 +1,18 @@
> +.text

Please can you get used to not using unindented directives?

> +	#ttdpbf16ps %tmm1, %tmm1, %tmm2 all tmm registers should be distinct
> +	.insn VEX.128.f3.0F38.W0 0x6c, %tmm1, %tmm1, %tmm2
> +
> +	#ttdpbf16ps %tmm1, %tmm2, %tmm1 all tmm registers should be distinct
> +	.insn VEX.128.f3.0F38.W0 0x6c, %tmm1, %tmm2, %tmm1
> +
> +	#ttdpbf16ps %tmm2, %tmm1, %tmm1 all tmm registers should be distinct
> +	.insn VEX.128.f3.0F38.W0 0x6c, %tmm2, %tmm1, %tmm1
> +
> +	#ttdpfp16ps %tmm1, %tmm1, %tmm2 all tmm registers should be distinct
> +	.insn VEX.128.f2.0F38.W0 0x6c, %tmm1, %tmm1, %tmm2
> +
> +	#ttdpfp16ps %tmm1, %tmm2, %tmm1 all tmm registers should be distinct
> +	.insn VEX.128.f2.0F38.W0 0x6c, %tmm1, %tmm2, %tmm1
> +
> +	#ttdpfp16ps %tmm2, %tmm1, %tmm1 all tmm registers should be distinct
> +	.insn VEX.128.f2.0F38.W0 0x6c, %tmm2, %tmm1, %tmm1

Please put a blank each after the #-es.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-amx-transpose-inval.s
> @@ -0,0 +1,18 @@
> +# Check Illegal AMX-TRANSPOSE instructions
> +
> +	.text
> +_start:
> +	ttdpbf16ps	%tmm1, %tmm1, %tmm2
> +	ttdpbf16ps	%tmm1, %tmm2, %tmm1
> +	ttdpbf16ps	%tmm2, %tmm1, %tmm1
> +	ttdpfp16ps	%tmm1, %tmm1, %tmm2
> +	ttdpfp16ps	%tmm1, %tmm2, %tmm1
> +	ttdpfp16ps	%tmm2, %tmm1, %tmm1
> +	t2rpntlvwz0	(%rip), %tmm1
> +	t2rpntlvwz0t1	(%rip), %tmm1
> +	t2rpntlvwz1	(%rip), %tmm1
> +	t2rpntlvwz1t1	(%rip), %tmm1

As odd numbers might be warned about no matter of the %rip-related error,
please use even numbered destination registers here. (Underlying principle:
Only one invalid / bogus item at a time.)

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3210,6 +3210,22 @@ tilerelease, 0x49c0, AMX_TILE, Vex128|Space0F38|VexW0|NoSuf, {}
>  
>  tilezero, 0xf249, AMX_TILE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM }
>  
> +t2rpntlvwz0, 0x6e, APX_F(AMX_TRANSPOSE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz0t1, 0x6f, APX_F(AMX_TRANSPOSE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz1, 0x666e, APX_F(AMX_TRANSPOSE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz1t1, 0x666f, APX_F(AMX_TRANSPOSE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }

Considering the AMX-MOVRS counterparts that are going to appear I think
it would make sense to templatize these (twice, i.e. something like
t2rpntlvwz<z><l> or t2rpntlvw<z><loc> or some such).

Jan