[PATCH 2/6] Support Intel AMX-AVX512

[Jan Beulich]

On 13.11.2024 09:44, Haochen Jiang wrote:
> --- a/gas/NEWS
> +++ b/gas/NEWS
> @@ -1,5 +1,7 @@
>  -*- text -*-
>  
> +* Add support for Intel AMX-AVX512 instructions.
> +
>  * Add support for Intel AMX-TRANSPOSE instructions.

As they're closely related, I think all AMX-* together want to take just a
single list entry.

> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -547,6 +547,7 @@ if [gas_32_check] then {
>      run_dump_test "avx10_2-256-miscs-intel"
>      run_list_test "msr_imm-inval"
>      run_list_test "amx-transpose-inval"
> +    run_list_test "amx-avx512-inval"

See comment on earlier patch.

> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -592,6 +592,7 @@ fetch_error (const instr_info *ins)
>  #define VexGatherD { OP_VEX, vex_vsib_d_w_dq_mode }
>  #define VexGatherQ { OP_VEX, vex_vsib_q_w_dq_mode }
>  #define VexGdq { OP_VEX, dq_mode }
> +#define VexGd { OP_VEX, d_mode }

Why wouldn't VexGdq be suitable to use?

> @@ -13931,6 +13949,8 @@ OP_VEX (instr_info *ins, int bytemode, int sizeflag ATTRIBUTE_UNUSED)
>      case 512:
>        names = att_names_zmm;
>        ins->evex_used |= EVEX_len_used;
> +      if (bytemode == d_mode)
> +	names = att_names32;
>        break;
>      default:
>        abort ();

Irrespective of VexGd (i.e. d_mode) or VexGdq (dq_mode) - the GPR handling
imo simply wants pulling out of this switch().

> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -265,6 +265,8 @@ static const dependency isa_dependencies[] =
>      "AMX_TILE" },
>    { "AMX_TRANSPOSE",
>      "AMX_TILE" },
> +  { "AMX_AVX512",
> +    "AMX_TILE|AVX10_2" },

This dependency looks certainly correct to add, yet how does that fit with
all insns only supporting VL=512, when AVX10 is specifically about permitting
vector lengths only up to 256 in hardware?

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3204,6 +3204,19 @@ tconjtcmmimfp16ps, 0x6b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2V
>  
>  tconjtfp16, 0x666b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM, RegTMM }
>  
> +tcvtrowd2ps, 0xf34a, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tcvtrowd2ps, 0xf307, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
> +
> +tcvtrowps2pbf16h, 0xf26d, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tcvtrowps2pbf16h, 0xf207, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
> +tcvtrowps2pbf16l, 0xf36d, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tcvtrowps2pbf16l, 0xf377, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
> +
> +tcvtrowps2phh, 0x6d, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tcvtrowps2phh, 0x07, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
> +tcvtrowps2phl, 0x666d, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tcvtrowps2phl, 0xf277, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
> +
>  tdpbf16ps, 0xf35c, AMX_BF16, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  tdpfp16ps, 0xf25c, AMX_FP16, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  tdpbssd, 0xf25e, AMX_INT8, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> @@ -3213,6 +3226,8 @@ tdpbsud, 0xf35e, AMX_INT8, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM
>  
>  tileloadd, 0xf24b, APX_F(AMX_TILE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
>  tileloaddt1, 0x664b, APX_F(AMX_TILE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +tilemovrow, 0x664a, AMX_AVX512, Modrm|EVex512|Space0F38|Src2VVVV|VexW0|NoSuf, { Reg32, RegTMM, RegZMM }
> +tilemovrow, 0x6607, AMX_AVX512, Modrm|EVex512|Space0F3A|VexW0|NoSuf, { Imm8, RegTMM, RegZMM }
>  tilestored, 0xf34b, APX_F(AMX_TILE), Sibmem|Vex128|EVex128|Space0F38|VexW0|NoSuf, { RegTMM, Unspecified|BaseIndex }
>  
>  tilerelease, 0x49c0, AMX_TILE, Vex128|Space0F38|VexW0|NoSuf, {}

Again I'm wondering why the additions are scattered around, rather than kept
together (and not going in the middle of other sub-groups). Hmm, now that I
look at this a 3rd time - is this perhaps an attempt to sort alphabetically?
Such sorting is imo fine as a secondary criteria; the first imo ought to be
the feature.

And just to mention it here again - this shouldn't go in without the encoding
anomalies sorted, one way or the other.

Jan