[PATCH 1/6] Support Intel AMX-TRANSPOSE

Jan Beulich jbeulich@suse.com
Fri Nov 15 13:40:05 GMT 2024


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-TRANSPOSE instructions.

As before, preferably with "x86" added, please.

> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -1182,6 +1182,7 @@ static const arch_entry cpu_arch[] =
>    SUBARCH (amx_bf16, AMX_BF16, ANY_AMX_BF16, false),
>    SUBARCH (amx_fp16, AMX_FP16, ANY_AMX_FP16, false),
>    SUBARCH (amx_complex, AMX_COMPLEX, ANY_AMX_COMPLEX, false),
> +  SUBARCH (amx_transpose, AMX_TRANSPOSE, ANY_AMX_TRANSPOSE, false),
>    SUBARCH (amx_tile, AMX_TILE, ANY_AMX_TILE, false),
>    SUBARCH (movdiri, MOVDIRI, MOVDIRI, false),
>    SUBARCH (movdir64b, MOVDIR64B, MOVDIR64B, false),
> @@ -1858,6 +1859,7 @@ _is_cpu (const i386_cpu_attr *a, enum i386_cpu cpu)
>      case CpuAVX512F:  return a->bitfield.cpuavx512f;
>      case CpuAVX512VL: return a->bitfield.cpuavx512vl;
>      case CpuAPX_F:    return a->bitfield.cpuapx_f;
> +    case CpuAMX_TRANSPOSE:  return a->bitfield.cpuamx_transpose;

Nit: One too many padding blanks.

> @@ -10977,7 +10979,7 @@ build_modrm_byte (void)
>      {
>        if (i.mem_operands)
>  	{
> -	  unsigned int fake_zero_displacement = 0;
> +	  unsigned int fake_zero_displacement = 0, tmmpair = 0, pos = 0;
>  
>  	  gas_assert (i.flags[op] & Operand_Mem);
>  
> @@ -11009,6 +11011,20 @@ build_modrm_byte (void)
>  		    i.sib.index = i.index_reg->reg_num;
>  		  set_rex_vrex (i.index_reg, REX_X, false);
>  		}
> +
> +	      /* Since some amx instructions uses tmm pairs, which will
> +		 automatically change tmm with odd number to even number.
> +		 So we will handle this here.  */
> +	      tmmpair = i.tm.opcode_modifier.tmmpairoperand & 7;
> +	      while (tmmpair)
> +		{
> +		  if (tmmpair % 2 == 1
> +		      && i.op[pos].regs->reg_num % 2 == 1)
> +		    i.op[pos].regs--;
> +		  tmmpair >>= 1;
> +		  pos++;
> +		} 
> +
>  	    }

Besides you wanting to re-use what we already have, we also want to be
consistent in how we handle this: For the other cases we don't adjust the
register; we merely warn about the anomaly. Same should then be happening
for this case.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/amx-transpose-inval.l
> @@ -0,0 +1,12 @@
> +.* Assembler messages:
> +.*:6: Error: `ttdpbf16ps' is only supported in 64-bit mode
> +.*:7: Error: `ttdpfp16ps' is only supported in 64-bit mode
> +.*:8: Error: `ttransposed' is only supported in 64-bit mode
> +.*:9: Error: `t2rpntlvwz0' is only supported in 64-bit mode
> +.*:10: Error: `t2rpntlvwz0t1' is only supported in 64-bit mode
> +.*:11: Error: `t2rpntlvwz1' is only supported in 64-bit mode
> +.*:12: Error: `t2rpntlvwz1t1' is only supported in 64-bit mode
> +.*:13: Error: `tconjtcmmimfp16ps' is only supported in 64-bit mode
> +.*:14: Error: `tconjtfp16' is only supported in 64-bit mode
> +.*:15: Error: `ttcmmimfp16ps' is only supported in 64-bit mode
> +.*:16: Error: `ttcmmrlfp16ps' is only supported in 64-bit mode

I question the value of this test (and similar ones, especially when the
base feature already isn't permitted outside of 64-bit mode).

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-amx-transpose.d
> @@ -0,0 +1,31 @@
> +#objdump: -dw
> +#name: x86_64 AMX-TRANSPOSE insns
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <_start>:
> +\s*[a-f0-9]+:\s*c4 e2 5a 6c f5\s+ttdpbf16ps %tmm4,%tmm5,%tmm6
> +\s*[a-f0-9]+:\s*c4 e2 72 6c da\s+ttdpbf16ps %tmm1,%tmm2,%tmm3
> +\s*[a-f0-9]+:\s*c4 e2 5b 6c f5\s+ttdpfp16ps %tmm4,%tmm5,%tmm6
> +\s*[a-f0-9]+:\s*c4 e2 73 6c da\s+ttdpfp16ps %tmm1,%tmm2,%tmm3
> +\s*[a-f0-9]+:\s*c4 e2 7a 5f f5\s+ttransposed %tmm5,%tmm6
> +\s*[a-f0-9]+:\s*c4 e2 7a 5f da\s+ttransposed %tmm2,%tmm3
> +\s*[a-f0-9]+:\s*c4 a2 78 6e b4 f5 00 00 00 10\s+t2rpntlvwz0 0x10000000\(%rbp,%r14,8\),%tmm6
> +\s*[a-f0-9]+:\s*c4 c2 78 6e 14 21\s+t2rpntlvwz0 \(%r9,%riz,1\),%tmm2
> +\s*[a-f0-9]+:\s*c4 a2 78 6f b4 f5 00 00 00 10\s+t2rpntlvwz0t1 0x10000000\(%rbp,%r14,8\),%tmm6
> +\s*[a-f0-9]+:\s*c4 c2 78 6f 14 21\s+t2rpntlvwz0t1 \(%r9,%riz,1\),%tmm2
> +\s*[a-f0-9]+:\s*c4 a2 79 6e b4 f5 00 00 00 10\s+t2rpntlvwz1 0x10000000\(%rbp,%r14,8\),%tmm6
> +\s*[a-f0-9]+:\s*c4 c2 79 6e 14 21\s+t2rpntlvwz1 \(%r9,%riz,1\),%tmm2
> +\s*[a-f0-9]+:\s*c4 a2 79 6f b4 f5 00 00 00 10\s+t2rpntlvwz1t1 0x10000000\(%rbp,%r14,8\),%tmm6
> +\s*[a-f0-9]+:\s*c4 c2 79 6f 14 21\s+t2rpntlvwz1t1 \(%r9,%riz,1\),%tmm2

With what I said above, the use of %tmm3 in the source file should
result in %tmm3 being displayed here. As mentioned in the series extending
the group handling, we ought to think about how to express odd registers in
disassembly. Ideally that would happen before the issue is widened by this
introducing further instances.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3183,14 +3183,27 @@ xresldtrk, 0xf20f01e9, TSXLDTRK, NoSuf, {}
>  
>  // TSXLDTRK instructions end.
>  
> +#define TMMPairOperand0 TMMPairOperand=1
> +#define TMMPairOperand1 TMMPairOperand=2
> +#define TMMPairOperand2 TMMPairOperand=4
> +
>  // AMX instructions.
>  
>  ldtilecfg, 0x49/0, APX_F(AMX_TILE), Modrm|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex }
>  sttilecfg, 0x6649/0, APX_F(AMX_TILE), Modrm|Vex128|EVex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex }
>  
> +t2rpntlvwz0, 0x6e, AMX_TRANSPOSE, TMMPairOperand1|Sibmem|Vex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz0t1, 0x6f, AMX_TRANSPOSE, TMMPairOperand1|Sibmem|Vex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz1, 0x666e, AMX_TRANSPOSE, TMMPairOperand1|Sibmem|Vex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +t2rpntlvwz1t1, 0x666f, AMX_TRANSPOSE, TMMPairOperand1|Sibmem|Vex128|Space0F38|VexW0|NoSuf, { Unspecified|BaseIndex, RegTMM }
> +
>  tcmmimfp16ps, 0x666c, AMX_COMPLEX, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  tcmmrlfp16ps, 0x6c, AMX_COMPLEX, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
>  
> +tconjtcmmimfp16ps, 0x6b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> +
> +tconjtfp16, 0x666b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM, RegTMM }
> +
>  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 }
> @@ -3206,6 +3219,14 @@ tilerelease, 0x49c0, AMX_TILE, Vex128|Space0F38|VexW0|NoSuf, {}
>  
>  tilezero, 0xf249, AMX_TILE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM }
>  
> +ttcmmimfp16ps, 0xf26b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> +ttcmmrlfp16ps, 0xf36b, AMX_COMPLEX&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> +
> +ttdpbf16ps, 0xf36c, AMX_BF16&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> +ttdpfp16ps, 0xf26c, AMX_FP16&AMX_TRANSPOSE, Modrm|Vex128|Space0F38|Src2VVVV|VexW0|NoSuf, { RegTMM, RegTMM, RegTMM }
> +
> +ttransposed, 0xf35f, AMX_TRANSPOSE, Modrm|Vex128|Space0F38|VexW0|NoSuf, { RegTMM, RegTMM }
> +
>  // AMX instructions end.

I'm struggling some in trying to determine on what basis you've established
where to add the new insns. Would imo be nice if all AMX-COMPLEX ones ended
up together, all AMX-BF16 etc. Or alternatively if all AMX-TRANSPOSE ones
ended up together (and not at the very top of the section).

Jan


More information about the Binutils mailing list