linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andy Lutomirski <luto@amacapital.net>, X86 ML <x86@kernel.org>
Subject: Re: [PATCH 2/2] x86/insn: perf tools: Add AVX-512 support to the instruction decoder
Date: Wed, 20 Jul 2016 11:33:00 +0300	[thread overview]
Message-ID: <578F373C.5000809@intel.com> (raw)
In-Reply-To: <20160720105033.43d4553550031e5dd33d5f9d@kernel.org>

On 20/07/16 04:50, Masami Hiramatsu wrote:
> 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,
> 
> 

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

  reply	other threads:[~2016-07-20  8:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-19 12:46 [PATCH 0/2] x86/insn: perf tools: Add AVX-512 support to the instruction decoder Adrian Hunter
2016-07-19 12:46 ` [PATCH 1/2] x86/insn: perf tools: Fix vcvtph2ps instruction decoding Adrian Hunter
2016-07-20  0:35   ` Masami Hiramatsu
2016-07-19 12:46 ` [PATCH 2/2] x86/insn: perf tools: Add AVX-512 support to the instruction decoder Adrian Hunter
2016-07-20  1:50   ` Masami Hiramatsu
2016-07-20  8:33     ` Adrian Hunter [this message]
2016-07-20  0:23 ` [PATCH 0/2] " Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=578F373C.5000809@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).