public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: "Jin, Yao" <yao.jin@linux•intel.com>
To: Michael Ellerman <mpe@ellerman•id.au>,
	acme@kernel•org, jolsa@kernel•org, peterz@infradead•org,
	mingo@redhat•com, alexander.shishkin@linux•intel.com
Cc: ak@linux•intel.com, kan.liang@intel•com,
	linuxppc-dev@lists•ozlabs.org, Linux-kernel@vger•kernel.org,
	yao.jin@intel•com
Subject: Re: [PATCH v6 1/7] perf/core: Define the common branch type classification
Date: Mon, 10 Jul 2017 16:16:13 +0800	[thread overview]
Message-ID: <820424b8-d7b3-56cc-2b97-ec570d44ec25@linux.intel.com> (raw)
In-Reply-To: <87r2xoj08g.fsf@concordia.ellerman.id.au>



On 7/10/2017 2:05 PM, Michael Ellerman wrote:
> Hi Jin Yao,
>
> Sorry I haven't commented until now, but it got lost in the flood of
> patches.

Never mind, it's no problem. :)

> Just a few nit-picks below ...
> Jin Yao <yao.jin@linux•intel.com> writes:
>
>> It is often useful to know the branch types while analyzing branch
>> data. For example, a call is very different from a conditional branch.
>>
>> Currently we have to look it up in binary while the binary may later
>> not be available and even the binary is available but user has to take
>> some time. It is very useful for user to check it directly in perf
>> report.
>>
>> Perf already has support for disassembling the branch instruction
>> to get the x86 branch type.
>>
>> To keep consistent on kernel and userspace and make the classification
>> more common, the patch adds the common branch type classification
>> in perf_event.h.
> Most of the code and doc uses "branch" but then a few these are called
> "jump". Can we just stick with "branch"?
>
>> PERF_BR_NONE      : unknown
>> PERF_BR_JCC       : conditional jump
>> PERF_BR_JMP       : jump
>> PERF_BR_IND_JMP   : indirect jump
> eg:
>
> PERF_BR_COND    : conditional branch
> PERF_BR_UNCOND  : unconditional branch
> PERF_BR_IND     : indirect branch

Call and jump are all branches. If we want to figure out which one is 
jump and which one is call, we need the detail branch type definitions.

For example,  if we only say "PERF_BR_IND", we could not know if it's an 
indirect jump or indirect call.
>> PERF_BR_CALL      : call
>> PERF_BR_IND_CALL  : indirect call
>> PERF_BR_RET       : return
>> PERF_BR_SYSCALL   : syscall
>> PERF_BR_SYSRET    : syscall return
>> PERF_BR_IRQ       : hw interrupt/trap/fault
>> PERF_BR_INT       : sw interrupt
> I'm not sure what that means, I'm guessing on x86 it means someone
> executed "int" ?

PERF_BR_IRQ is for hw interrupt and PERF_BR_INT is for sw interrupt.

PERF_BR_CALL/PERF_BR_IND_CALL and PERF_BR_RET are for function call 
(direct call and indirect call) and return.

PERF_BR_SYSCALL/PERF_BR_SYSRET are for syscall and syscall return.

> Is that sufficiently useful to use up a bit? I think we only have 3
> free?

Do you means 3 bits? Each bit stands for one branch type? I guess what 
you mean is:

PERF_BR_COND    : conditional branch
PERF_BR_UNCOND  : unconditional branch
PERF_BR_IND     : indirect branch

But 3 branch types are not enough for us.

>> PERF_BR_IRET      : return from interrupt
>> PERF_BR_FAR_BRANCH: not generic far branch type
> What is a "not generic far branch" ?
>
> I don't know what that would mean on powerpc for example.

It's reserved for future using I think.

>
> I think the only thing we have on powerpc that's commonly used and that
> isn't covered above is branches that decrement a loop counter and then
> branch based on the result.
>
> It might be nice if we could separate those out from other conditional
> branches. Whether it's worth using a bit for I'm not sure. Do other
> arches have something similar?
>
> Those branches do tend to be "backward conditional", so that may be
> sufficient. But backward conditional also includes if bodies that have
> been moved out of line and then branch back to the main body of the
> function.
>
> cheers

Sorry, I'm not familiar with powerpc arch. Or could you add the branch 
type which powerpc needs?

For backward conditional and forward conditional, we compute them in 
userspace according to the from/to addresses.

Thanks
Jin Yao

  reply	other threads:[~2017-07-10  8:16 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-20 12:07 [PATCH v6 0/7] perf report: Show branch type Jin Yao
2017-04-20  9:36 ` Jiri Olsa
2017-04-23  8:36   ` Jin, Yao
2017-06-02  8:02   ` Jin, Yao
2017-06-26  6:24     ` Jin, Yao
2017-07-06  1:47       ` Jin, Yao
2017-04-20 12:07 ` [PATCH v6 1/7] perf/core: Define the common branch type classification Jin Yao
2017-07-07  8:42   ` Peter Zijlstra
2017-07-10  5:19     ` Michael Ellerman
2017-07-10  6:05   ` Michael Ellerman
2017-07-10  8:16     ` Jin, Yao [this message]
2017-07-10 10:32       ` Michael Ellerman
2017-07-10 11:46         ` Jin, Yao
2017-07-10 13:10           ` Segher Boessenkool
2017-07-10 13:28             ` Jin, Yao
2017-07-10 13:46             ` Peter Zijlstra
2017-07-10 14:06               ` Jin, Yao
2017-07-11  2:28                 ` Michael Ellerman
2017-07-11  3:00                   ` Jin, Yao
2017-07-10 14:37               ` Segher Boessenkool
2017-07-11  2:13             ` Michael Ellerman
2017-04-20 12:07 ` [PATCH v6 2/7] perf/x86/intel: Record branch type Jin Yao
2017-04-23 13:55   ` Jiri Olsa
2017-04-24  0:47     ` Jin, Yao
2017-05-08  0:49       ` Jin, Yao
2017-05-09  8:26       ` Jiri Olsa
2017-05-09 11:57         ` Jin, Yao
2017-05-09 12:39           ` Jiri Olsa
2017-05-10  0:18             ` Jin, Yao
2017-04-20 12:07 ` [PATCH v6 3/7] perf record: Create a new option save_type in --branch-filter Jin Yao
2017-04-20 12:07 ` [PATCH v6 4/7] perf report: Refactor the branch info printing code Jin Yao
2017-04-20 12:07 ` [PATCH v6 5/7] perf util: Create branch.c/.h for common branch functions Jin Yao
2017-04-20 12:07 ` [PATCH v6 6/7] perf report: Show branch type statistics for stdio mode Jin Yao
2017-04-20 12:07 ` [PATCH v6 7/7] perf report: Show branch type in callchain entry Jin Yao
2017-07-07  8:09 ` [PATCH v6 0/7] perf report: Show branch type Jiri Olsa

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=820424b8-d7b3-56cc-2b97-ec570d44ec25@linux.intel.com \
    --to=yao.jin@linux$(echo .)intel.com \
    --cc=Linux-kernel@vger$(echo .)kernel.org \
    --cc=acme@kernel$(echo .)org \
    --cc=ak@linux$(echo .)intel.com \
    --cc=alexander.shishkin@linux$(echo .)intel.com \
    --cc=jolsa@kernel$(echo .)org \
    --cc=kan.liang@intel$(echo .)com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=mingo@redhat$(echo .)com \
    --cc=mpe@ellerman$(echo .)id.au \
    --cc=peterz@infradead$(echo .)org \
    --cc=yao.jin@intel$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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