From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v3 1/2] perf util: Check for fused instruction
Date: Fri, 7 Jul 2017 09:09:51 +0800 [thread overview]
Message-ID: <304bb1d4-6d31-ecaa-1a08-ddb7c5cc6e7a@linux.intel.com> (raw)
In-Reply-To: <20170706162402.GL27350@kernel.org>
>> +
>> +static bool x86__ins_is_fused(char *cpuid, const char *ins1, const char *ins2)
>> +{
>> + unsigned int family, model, stepping;
>> + int ret;
>> +
>> + /*
>> + * cpuid = "GenuineIntel,family,model,stepping"
>> + */
>> + ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
> So, looking at the next patch, that uses this, I see that you'll call
> this everytime that jump arrow will be printed, why not do this when
> doing the initial disassembly, and having this info cached in the struct
> ins or disasm_line (haven't looked closely to provide exact instructions
> on how to do it)?
Yes, you're right. It doesn't need to do this model checking each time.
I will move the model checking to other place and record the model in
annotate_browser.
> Even more, you could do this model checking just once per disassembly,
> then use it as you go reading the disassembly lines, marking them as
> fused/not fused and then at jump arror printing just look at a flag, no?
>
> - Arnaldo
>
For this, I have another consideration.
The fused instruction pair consists of 2 instructions. One is jump, the
other is "CMP/TEST/...".
If I check the fused instruction pair once per disassembly, I need to
check if one instruction is jump and the other is "CMP/TEST/..." for all
instructions.
While in annotate_browser__draw_current_jump(), it has already checked
if a disasm_line is a valid jump, so the jump checking will be duplicated.
The current implementation is just performing the fused checking when
user moves the cursor on the jump instruction. It doesn't need to add
additional jump checking.
So the current way may be acceptable as well?
Thanks
Jin Yao
next prev parent reply other threads:[~2017-07-07 1:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-20 12:22 [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
2017-07-06 16:24 ` Arnaldo Carvalho de Melo
2017-07-07 1:09 ` Jin, Yao [this message]
2017-06-20 12:22 ` [PATCH v3 2/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-07-06 0:42 ` [PATCH v3 0/2] " Jin, Yao
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=304bb1d4-6d31-ecaa-1a08-ddb7c5cc6e7a@linux.intel.com \
--to=yao.jin@linux.intel.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
/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
Powered by JetHome