Re: [PATCH 2/2] x86/insn: perf tools: Add AVX-512 support to the instruction decoder

[Adrian Hunter]

On 20/07/16 04:50, Masami Hiramatsu wrote:
> On Tue, 19 Jul 2016 15:46:48 +0300
> Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> 
>> Add support for Intel's AVX-512 instructions to the instruction decoder.
>>
>> AVX-512 instructions are documented in Intel Architecture Instruction Set
>> Extensions Programming Reference (February 2016).
>>
>> AVX-512 instructions are identifed by a EVEX prefix which, for the purpose
>> of instruction decoding, can be treated as though it were a 4-byte VEX
>> prefix.
>>
>> Existing instructions which can now accept an EVEX prefix need not be
>> further annotated in the op code map (x86-opcode-map.txt). In the case of
>> new instructions, the op code map is updated accordingly.
>>
>> Also add associated Mask Instructions that are used to manipulate mask
>> registers used in AVX-512 instructions.
>>
>> Add a representative set of instructions to the perf tools new instructons
>> test.
> 
> Hmm, could you split this patch into 2 or 3 parts for review?
> One is for core-kernel, the others are for perf tools (feature
> adding and tests).
> 
> [..]
>> diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
>> index 28082de46f0d..92b89fa5f414 100644
>> --- a/arch/x86/lib/x86-opcode-map.txt
>> +++ b/arch/x86/lib/x86-opcode-map.txt
>> @@ -13,12 +13,17 @@
>>  # opcode: escape # escaped-name
>>  # EndTable
>>  #
>> +# mnemonics that begin with lowercase 'v' accept a VEX or EVEX prefix
>> +# mnemonics that begin with lowercase 'k' accept a VEX prefix
>> +#
> 
> Ah, nice :)
> 
>>  #<group maps>
>>  # GrpTable: GrpXXX
>>  # reg:  mnemonic [operand1[,operand2...]] [(extra1)[,(extra2)...] [| 2nd-mnemonic ...]
>>  # EndTable
>>  #
>>  # AVX Superscripts
>> +#  (ev): this opcode requires EVEX prefix.
>> +#  (evo): this opcode accepts EVEX prefix.
> 
> Hmm, what the 'o' stands for? I thought it means "EVEX ONLY", but the comment is
> opposite. For example, see 'o64', which means "64bit only".
> And anyway, as you said above, if all existing instructions can accept EVEX prefix,
> why would we need it?
> 
>> -5b: vcvtdq2ps Vps,Wdq | vcvtps2dq Vdq,Wps (66) | vcvttps2dq Vdq,Wps (F3)
>> +5b: vcvtdq2ps Vps,Wdq | vcvtqq2ps Vps,Wqq (evo) | vcvtps2dq Vdq,Wps (66) | vcvttps2dq Vdq,Wps (F3)
> 
> Ah, I see, so that is for the instructions which change the mnemonic since
> byte-width is changed...
> 
> OK, so please update the above description of the superscript, like as
> "this opcode is changed by EVEX prefix (EVEX opcode)" etc.
> 
> Thank you,
> 
> 

Thanks for your quick reply.  I have updated the patches as you requested
and sent V2.