[PATCH v3 1/2] Support Intel AMX-MOVRS

Thu Jan 9 13:05:29 GMT 2025

On 09.01.2025 07:00, Haochen Jiang wrote:
> gas/ChangeLog:
> 
> 	* NEWS: Support Intel AMX-MOVRS.

This ...

> 	* config/tc-i386.c: Add amx_movrs.
> 	* doc/c-i386.texi: Document .amx_movrs.
> 	* testsuite/gas/i386/x86-64.exp: Run AMX-MOVRS tests.
> 	* testsuite/gas/i386/x86-64-amx-movrs-intel.d: New test.
> 	* testsuite/gas/i386/x86-64-amx-movrs-inval.l: Ditto.
> 	* testsuite/gas/i386/x86-64-amx-movrs-inval.s: Ditto.
> 	* testsuite/gas/i386/x86-64-amx-movrs.d: Ditto.
> 	* testsuite/gas/i386/x86-64-amx-movrs.s: Ditto.
> 
> opcodes/ChangeLog:
> 
> 	* i386-dis-evex-len.h (EVEX_LEN_0F384A_X86_64_W_0): New.
> 	* i386-dis-evex-w.h (EVEX_W_0F384A_X86_64): Ditto.
> 	* i386-dis-evex-x86-64.h (X86_64_EVEX_0F384A): Ditto.
> 	* i386-dis-evex.h: New entry for AMX-MOVRS.
> 	* i386-dis.c:
> 	(MOD_VEX_MAP5_F8_X86_64): Ditto.
> 	(MOD_VEX_MAP5_F9_X86_64): Ditto.
> 	(PREFIX_VEX_0F384A_X86_64_M_0_L_0_W_0): Ditto.
> 	(PREFIX_VEX_MAP5_F8_X86_64_M_0_L_0_W_0): Ditto.
> 	(PREFIX_VEX_MAP5_F9_X86_64_M_0_L_0_W_0): Ditto.
> 	(X86_64_VEX_0F384A): Ditto.
> 	(X86_64_VEX_MAP5_F8): Ditto.
> 	(X86_64_VEX_MAP5_F9): Ditto.
> 	(X86_64_EVEX_0F384A): Ditto.
> 	(VEX_LEN_0F384A_X86_64_W_0): Ditto.
> 	(VEX_LEN_MAP5_F8_X86_64_M_0): Ditto.
> 	(VEX_LEN_MAP5_F9_X86_64_M_0): Ditto.
> 	(EVEX_LEN_0F384A_X86_64_W_0): Ditto.
> 	(VEX_W_0F384A_X86_64): Ditto.
> 	(VEX_W_MAP5_F8_X86_64): Ditto.
> 	(VEX_W_MAP5_F9_X86_64): Ditto.
> 	(EVEX_W_0F384A_X86_64): Ditto.
> 	(prefix_table): New entry for AMX-MOVRS.
> 	(x86_64_table): Ditto.
> 	(vex_len_table): Ditto.
> 	(vex_w_table): Ditto.
> 	(map5_f8_opcode): New.
> 	(map5_f9_opcode): Ditto.
> 	(get_valid_dis386): Handle VEX_MAP5 opcode for AMX-MOVRS.
> 	* i386-gen.c (isa_dependencies): Add AMX_MOVRS.
> 	(cpu_flags): Ditto.
> 	* i386-init.h: Regenerated.
> 	* i386-mnem.h: Ditto.
> 	* i386-opc.h (CpuAMX_MOVRS): New.
> 	(i386_cpu_flags): Add cpuamx_movrs.
> 	* i386-opc.tbl: Add AMX-MOVRS instructions.
> 	* i386-tbl.h: Regenerated.
> ---
>  gas/config/tc-i386.c                          |   16 +-
>  gas/doc/c-i386.texi                           |    3 +-
>  .../gas/i386/x86-64-amx-movrs-intel.d         |   23 +
>  .../gas/i386/x86-64-amx-movrs-inval.l         |   13 +
>  .../gas/i386/x86-64-amx-movrs-inval.s         |   19 +
>  gas/testsuite/gas/i386/x86-64-amx-movrs.d     |   21 +
>  gas/testsuite/gas/i386/x86-64-amx-movrs.s     |   31 +
>  .../gas/i386/x86-64-apx-evex-promoted-intel.d |   16 +
>  .../gas/i386/x86-64-apx-evex-promoted-wig.d   |   16 +
>  .../gas/i386/x86-64-apx-evex-promoted.d       |   16 +
>  .../gas/i386/x86-64-apx-evex-promoted.s       |   16 +
>  gas/testsuite/gas/i386/x86-64.exp             |    3 +
>  opcodes/i386-dis-evex-len.h                   |    5 +
>  opcodes/i386-dis-evex-w.h                     |    4 +
>  opcodes/i386-dis-evex-x86-64.h                |    5 +
>  opcodes/i386-dis-evex.h                       |    6 +-
>  opcodes/i386-dis.c                            |  108 +-
>  opcodes/i386-gen.c                            |    3 +
>  opcodes/i386-init.h                           |  742 +--
>  opcodes/i386-mnem.h                           | 4350 +++++++++--------
>  opcodes/i386-opc.h                            |    3 +
>  opcodes/i386-opc.tbl                          |   11 +-
>  opcodes/i386-tbl.h                            |  279 +-
>  23 files changed, 3058 insertions(+), 2651 deletions(-)
>  create mode 100644 gas/testsuite/gas/i386/x86-64-amx-movrs-intel.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-amx-movrs-inval.l
>  create mode 100644 gas/testsuite/gas/i386/x86-64-amx-movrs-inval.s
>  create mode 100644 gas/testsuite/gas/i386/x86-64-amx-movrs.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-amx-movrs.s
>  create mode 100644 opcodes/i386-dis-evex-x86-64.h

... must have been lost, which is even more so odd since this - if I'm
not mistaken - not only adds full AMX-MOVRS support, but also completes
AMX-TRANSPOSE.

> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -1185,6 +1185,7 @@ static const arch_entry cpu_arch[] =
>    SUBARCH (amx_transpose, AMX_TRANSPOSE, ANY_AMX_TRANSPOSE, false),
>    SUBARCH (amx_tf32, AMX_TF32, ANY_AMX_TF32, false),
>    SUBARCH (amx_fp8, AMX_FP8, ANY_AMX_FP8, false),
> +  SUBARCH (amx_movrs, AMX_MOVRS, ANY_AMX_MOVRS, false),
>    SUBARCH (amx_tile, AMX_TILE, ANY_AMX_TILE, false),
>    SUBARCH (movdiri, MOVDIRI, MOVDIRI, false),
>    SUBARCH (movdir64b, MOVDIR64B, MOVDIR64B, false),
> @@ -2249,14 +2250,22 @@ cpu_flags_match (const insn_template *t)
>  	      || any.bitfield.cpuavx512f || any.bitfield.cpuavx512bw
>  	      || any.bitfield.cpuavx512dq || any.bitfield.cpuamx_tile
>  	      || any.bitfield.cpucmpccxadd || any.bitfield.cpuuser_msr
> -	      || any.bitfield.cpumsr_imm || any.bitfield.cpuamx_transpose))
> +	      || any.bitfield.cpumsr_imm || any.bitfield.cpuamx_transpose
> +	      || any.bitfield.cpuamx_movrs))
>  	{
>  	  /* These checks (verifying that APX_F() was properly used in the
>  	     opcode table entry) make sure there's no need for an "else" to
>  	     the "if()" below.  */
>  	  gas_assert (!cpu_flags_all_zero (&all));
>  	  cpu = cpu_flags_and (all, any);
> -	  gas_assert (cpu_flags_equal (&cpu, &all));
> +	  /* For APX_F extension of multiple cpuid enabled insns, we could not
> +	     use APX_F(cpuid_A&couid_B) since the transformation could not be
> +	     done. Instead, we will use cpuid_A && APX_F(cpuid_B), then the
> +	     bitfield will not be match for cpu and all. Since there won't
> +	     be much cases, here we work around those CPUIDs and do not
> +	     need this assert check.  */
> +	  if (!(all.bitfield.cpuamx_transpose && any.bitfield.cpuamx_movrs))
> +	    gas_assert (cpu_flags_equal (&cpu, &all));

First: With the extensive comment, this workaround may be okay for now. One
nit though: s/much/many/.

However, slightly better would be if the assertion could remain in effect
even for these insns. Can we massage "all" (overwritten right afterwards
for the EVEX case) and/or "any" (cleared altogether afterwards) such that
the assertion would pass (and the resulting "cpu" still be correct)? Can
we perhaps even arrange for the adjustment to be done in i386-gen.c? (If
the answer is "no" for now, I can live with that. I'd then try to find
time later to clean this up.)

> @@ -4099,6 +4117,14 @@ static const struct dis386 prefix_table[][4] = {
>      { RM_TABLE (RM_VEX_0F3849_X86_64_L_0_W_0_M_1_P_3) },
>    },
>  
> +  /* PREFIX_VEX_0F384A_X86_64_W_0_L_0 */
> +  {
> +    { Bad_Opcode },
> +    { Bad_Opcode },
> +    { "tileloaddrst1",	{ TMM, MVexSIBMEM }, 0 },
> +    { "tileloaddrs",	{ TMM, MVexSIBMEM }, 0 },
> +  },

Seeing how this doesn't need to go through mod_table[], ...

> @@ -4283,6 +4309,20 @@ static const struct dis386 prefix_table[][4] = {
>      { "%XErorxS",		{ Gdq, Edq, Ib }, 0 },
>    },
>  
> +  /* PREFIX_VEX_MAP5_F8_X86_64_M_0_L_0_W_0 */
> +  {
> +    { "t2rpntlvwz0rs",	{ TMM, MVexSIBMEM }, 0 },
> +    { Bad_Opcode },
> +    { "t2rpntlvwz1rs",	{ TMM, MVexSIBMEM }, 0 },
> +  },
> +
> +  /* PREFIX_VEX_MAP5_F9_X86_64_M_0_L_0_W_0 */
> +  {
> +    { "t2rpntlvwz0rst1",	{ TMM, MVexSIBMEM }, 0 },
> +    { Bad_Opcode },
> +    { "t2rpntlvwz1rst1",	{ TMM, MVexSIBMEM }, 0 },
> +  },

... why do these need to? MVexSIBMEM expands to OP_M after all.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3216,9 +3216,6 @@ tilezero, 0xf249, AMX_TILE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM }
>  
>  t2rpntlvw<z><loc>, 0x<z:pfx>6e | <loc:opc>, APX_F(AMX_TRANSPOSE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }
>  
> -<z>
> -<loc>
> -

Can these stay where they are and instead ...

> @@ -3238,6 +3235,14 @@ tdpbhf8ps, 0xf2fd, AMX_FP8, Modrm|Vex128|Map5|Src2VVVV|VexW0|NoSuf, { RegTMM, Re
>  tdphbf8ps, 0xf3fd, AMX_FP8, Modrm|Vex128|Map5|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  tdphf8ps, 0x66fd, AMX_FP8, Modrm|Vex128|Map5|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  
> +t2rpntlvw<z>rs<loc>, 0x<z:pfx>f8 | <loc:opc>, AMX_TRANSPOSE&APX_F(AMX_MOVRS), Sibmem|Vex128|EVex128|Map5|VexW0|NoSuf|ImplicitGroup, { Unspecified|BaseIndex, RegTMM }

... this one move up? It can, after call be grouped with the TRANSPOSE ones as
well as with ...

> +<z>
> +<loc>
> +
> +tileloaddrs, 0xf24a, APX_F(AMX_MOVRS), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +tileloaddrst1, 0x664a, APX_F(AMX_MOVRS), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }

... the MOVRS ones, yet it's closer related to the earlier one (already mnemonmic-
wise).

Jan