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

[Masami Hiramatsu <mhiramat@kernel.org>]

On Tue, 19 Jul 2016 15:46:48 +0300
Adrian Hunter <adrian.hunter@intel.com> 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,


-- 
Masami Hiramatsu <mhiramat@kernel.org>