Skip to contentSkip to conversation

perf annotate: Support to display the LBR data in tui mode

PATCH Superseded Jin Yao 5 participants 12 messages Feb 23, 2018 – Mar 9, 2018 perf events

lore
Share & tools
lore

Jin Yao

author
#0 lore raw

PATCH 1/1 perf annotate: Support to display the LBR data in tui mode

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

This patch lets perf annotate support the displaying of LBR data.

For example,

perf annotate compute_flag

Percent│ IPC Cycle
       │
       │
       │                Disassembly of section .text:
       │
       │                0000000000400640 <compute_flag>:
       │                compute_flag():
       │                volatile int count;
       │                static unsigned int s_randseed;
       │
       │                __attribute__((noinline))
       │                int compute_flag()
       │                {
 22.96 │1.18   584        sub    $0x8,%rsp
       │                        int i;
       │
       │                        i = rand() % 2;
 23.02 │1.18     1      → callq  rand@plt
       │
       │                        return i;
 27.05 │3.37              mov    %eax,%edx
       │                }
       │3.37              add    $0x8,%rsp
       │                {
       │                        int i;
       │
       │                        i = rand() % 2;
       │
       │                        return i;
       │3.37              shr    $0x1f,%edx
       │3.37              add    %edx,%eax
       │3.37              and    $0x1,%eax
       │3.37              sub    %edx,%eax
       │                }
 26.97 │3.37     2      ← retq

Note that, this patch only supports tui mode. For other mode like --stdio, it just keeps original behavior.

perf annotate compute_flag --stdio

Percent | Source code & Disassembly of div for cycles:ppp (7993 samples)
------------------------------------------------------------------------------

         :
         :
         :
         :            Disassembly of section .text:
         :
         :            0000000000400640 <compute_flag>:
         :            compute_flag():
         :            volatile int count;
         :            static unsigned int s_randseed;
         :
         :            __attribute__((noinline))
         :            int compute_flag()
         :            {
    0.29 :   400640:       sub    $0x8,%rsp     # +100.00%
         :                    int i;
         :
         :                    i = rand() % 2;
   42.93 :   400644:       callq  400490 <rand@plt>     # -100.00% (p:100.00%)
         :
         :                    return i;
    0.10 :   400649:       mov    %eax,%edx     # +100.00%
         :            }
    0.94 :   40064b:       add    $0x8,%rsp
         :            {
         :                    int i;
         :
         :                    i = rand() % 2;
         :
         :                    return i;
   27.02 :   40064f:       shr    $0x1f,%edx
    0.15 :   400652:       add    %edx,%eax
    1.24 :   400654:       and    $0x1,%eax
    2.08 :   400657:       sub    %edx,%eax
         :            }

25.26 : 400659: retq # -100.00% (p:100.00%)

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/builtin-annotate.c | 88 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 6 deletions(-)
tools/perf/builtin-annotate.c +82 -6
index f15731a..ead6ae4 100644
@@ -44,6 +44,7 @@ struct perf_annotate {
 	bool	   full_paths;
 	bool	   print_line;
 	bool	   skip_missing;
+	bool	   has_br_stack;
 	const char *sym_hist_filter;
 	const char *cpu_list;
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);

@@ -146,16 +147,73 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
 	free(bi);
 }
 
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+				      struct addr_location *al __maybe_unused,
+				      bool single __maybe_unused,
+				      void *arg __maybe_unused)
+{
+	struct hist_entry *he = iter->he;
+	struct branch_info *bi;
+	struct perf_sample *sample = iter->sample;
+	struct perf_evsel *evsel = iter->evsel;
+	int err;
+
+	hist__account_cycles(sample->branch_stack, al, sample, false);
+
+	bi = he->branch_info;
+	err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx);
+
+	if (err)
+		goto out;
+
+	err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx);
+
+out:
+	return err;
+}
+
+static int process_branch_callback(struct perf_evsel *evsel,
+				   struct perf_sample *sample,
+				   struct addr_location *al __maybe_unused,
+				   struct perf_annotate *ann,
+				   struct machine *machine)
+{
+	struct hist_entry_iter iter = {
+		.evsel		= evsel,
+		.sample		= sample,
+		.add_entry_cb	= hist_iter__branch_callback,
+		.hide_unresolved	= symbol_conf.hide_unresolved,
+		.ops		= &hist_iter_branch,
+	};
+
+	struct addr_location a;
+	int ret;
+
+	if (machine__resolve(machine, &a, sample) < 0)
+		return -1;
+
+	if (a.sym == NULL)
+		return 0;
+
+	if (a.map != NULL)
+		a.map->dso->hit = 1;
+
+	ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann);
+	return ret;
+}
+
 static int perf_evsel__add_sample(struct perf_evsel *evsel,
 				  struct perf_sample *sample,
 				  struct addr_location *al,
-				  struct perf_annotate *ann)
+				  struct perf_annotate *ann,
+				  struct machine *machine)
 {
 	struct hists *hists = evsel__hists(evsel);
 	struct hist_entry *he;
 	int ret;
 
-	if (ann->sym_hist_filter != NULL &&
+	if ((!ann->has_br_stack || !ui__has_annotation()) &&
+	    ann->sym_hist_filter != NULL &&
 	    (al->sym == NULL ||
 	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
 		/* We're only interested in a symbol named sym_hist_filter */

@@ -178,6 +236,9 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 	 */
 	process_branch_stack(sample->branch_stack, al, sample);
 
+	if (ann->has_br_stack && ui__has_annotation())
+		return process_branch_callback(evsel, sample, al, ann, machine);
+
 	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;

@@ -206,7 +267,8 @@ static int process_sample_event(struct perf_tool *tool,
 	if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
 		goto out_put;
 
-	if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al, ann)) {
+	if (!al.filtered &&
+	    perf_evsel__add_sample(evsel, sample, &al, ann, machine)) {
 		pr_warning("problem incrementing symbol count, "
 			   "skipping event\n");
 		ret = -1;

@@ -238,6 +300,10 @@ static void hists__find_annotations(struct hists *hists,
 		if (he->ms.sym == NULL || he->ms.map->dso->annotate_warned)
 			goto find_next;
 
+		if (ann->sym_hist_filter &&
+		    (strcmp(he->ms.sym->name, ann->sym_hist_filter) != 0))
+			goto find_next;
+
 		notes = symbol__annotation(he->ms.sym);
 		if (notes->src == NULL) {
 find_next:

@@ -269,6 +335,7 @@ static void hists__find_annotations(struct hists *hists,
 			nd = rb_next(nd);
 		} else if (use_browser == 1) {
 			key = hist_entry__tui_annotate(he, evsel, NULL);
+
 			switch (key) {
 			case -1:
 				if (!ann->skip_missing)

@@ -489,6 +556,9 @@ int cmd_annotate(int argc, const char **argv)
 	if (annotate.session == NULL)
 		return -1;
 
+	annotate.has_br_stack = perf_header__has_feat(&annotate.session->header,
+						      HEADER_BRANCH_STACK);
+
 	ret = symbol__annotation_init();
 	if (ret < 0)
 		goto out_delete;

@@ -499,9 +569,6 @@ int cmd_annotate(int argc, const char **argv)
 	if (ret < 0)
 		goto out_delete;
 
-	if (setup_sorting(NULL) < 0)
-		usage_with_options(annotate_usage, options);
-
 	if (annotate.use_stdio)
 		use_browser = 0;
 	else if (annotate.use_tui)

@@ -511,6 +578,15 @@ int cmd_annotate(int argc, const char **argv)
 
 	setup_browser(true);
 
+	if (use_browser == 1 && annotate.has_br_stack) {
+		sort__mode = SORT_MODE__BRANCH;
+		if (setup_sorting(annotate.session->evlist) < 0)
+			usage_with_options(annotate_usage, options);
+	} else {
+		if (setup_sorting(NULL) < 0)
+			usage_with_options(annotate_usage, options);
+	}
+
 	ret = __cmd_annotate(&annotate);
 
 out_delete:
Signature · 1 line
2.7.4
maintainer
#1 lore raw
Quoted 7 lines · Jin Yao

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

author
#2 lore raw
Quoted 4 lines · Peter Zijlstra
Quoted 3 lines · Jin Yao

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

Quoted 8 lines
Quoted 4 lines

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

I will change the changelog to make it more clear.

Quoted 3 lines

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

I do plan to support stdio mode. While stdio mode needs more changes than tui mode, so I plan to do it in a follow-up patch.

Posting this patch now is because I want to listen from community first for this feature. If the tui patch could be accepted, then it's worth putting more efforts on stdio version. That's my thoughts.

Thanks
Jin Yao

maintainer
#3 lore raw
In reply to Peter Zijlstra

Em Fri, Feb 23, 2018 at 09:25:00AM +0100, Peter Zijlstra escreveu:

Quoted 7 lines · Jin Yao
Quoted 5 lines

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

Quoted 7 lines
Quoted 1 line

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

Right, Peter, what would you think if I made --stdio use the same routines used to format the TUI, i.e. stdio would be equal to the TUI modulo de navigation/jump arrows, etc.

We would have switches to provide the TUI output options that make sense for non-interactive mode, like:

  J   Toggle showing number of jump sources on targets
  o   Toggle disassembler output/simplified view
  s   Toggle source code view
  t   Circulate percent, total period, samples view 
  k   Toggle line numbers

And would still have e --stdio-classic (deprecated), that we would keep for a while.

I think that this new mode with "dissassembler output" would be the same as the current --stdio, I'll check.

To further clarify, this wouldn't use any ncurses/slang TUI code, just plain printf with things formatted using what is used now for the TUI mode.

This way there would never be any drift amongst the output modes and we would have less work to do when implementing new stuff like this LBR case.

What do you think?

- Arnaldo

maintainer
#4 lore raw
Quoted 13 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 23, 2018 at 09:25:00AM +0100, Peter Zijlstra escreveu:

Quoted 11 lines · Jin Yao
Quoted 7 lines

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Quoted 6 lines
Quoted 2 lines

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

Right, Peter, what would you think if I made --stdio use the same routines used to format the TUI, i.e. stdio would be equal to the TUI modulo de navigation/jump arrows, etc.

Ideally we'd share the whole lot between stdio/TUI/GUI. That said, I think stdio currently has a bunch of features that the other lack (my fault).

Quoted 8 lines

We would have switches to provide the TUI output options that make sense for non-interactive mode, like:

  J   Toggle showing number of jump sources on targets
  o   Toggle disassembler output/simplified view
  s   Toggle source code view
  t   Circulate percent, total period, samples view 
  k   Toggle line numbers

I really have no idea what you're talking about; this is because I've just _never_ seen TUI mode. I exclusively use stdio.

Quoted 2 lines

I think that this new mode with "dissassembler output" would be the same as the current --stdio, I'll check.

When I did the LBR coverage stuff I only did stdio; at the time we talked about merging all this further, and IIRC you said you had something like that on the TODO already so I left it there.

Quoted 3 lines

This way there would never be any drift amongst the output modes and we would have less work to do when implementing new stuff like this LBR case.

Yes, I think that's the right direction, but I fear there's quite a bit of work before we're at that point.

My only fear is that the resulting output code will be impenetrable, there's a reason I only ever touch stdio output :/

#5 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 13 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 23, 2018 at 09:25:00AM +0100, Peter Zijlstra escreveu:

Quoted 11 lines · Jin Yao
Quoted 7 lines

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Quoted 6 lines
Quoted 2 lines

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

Right, Peter, what would you think if I made --stdio use the same routines used to format the TUI, i.e. stdio would be equal to the TUI modulo de navigation/jump arrows, etc.

With the unification Jin Yao's patch would be the right thing anyways -- it would magically work for stdio mode too. So it seems to me merging the patch would be the right thing to do.

-Andi

author
#6 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 20 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 23, 2018 at 09:25:00AM +0100, Peter Zijlstra escreveu:

Quoted 7 lines · Jin Yao
Quoted 5 lines

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

Quoted 7 lines
Quoted 1 line

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

Right, Peter, what would you think if I made --stdio use the same routines used to format the TUI, i.e. stdio would be equal to the TUI modulo de navigation/jump arrows, etc.

Quoted 9 lines

We would have switches to provide the TUI output options that make sense for non-interactive mode, like:

   J   Toggle showing number of jump sources on targets
   o   Toggle disassembler output/simplified view
   s   Toggle source code view
   t   Circulate percent, total period, samples view
   k   Toggle line numbers

Hi Arnaldo, looks your idea is very similar as my idea. In my understanding, for example, we may provide switch to tui routine like annotate_browser__write() and use fprintf() to replace ui_browser__printf()/ui_browser_write__xxx() if switch is on for stdio.

Is that your idea?

For this approach, I think, the benefit is we can reuse most of existing code but the disadvantage is we have to mix tui and stdio up.

Thanks
Jin Yao

Quoted 18 lines

And would still have e --stdio-classic (deprecated), that we would keep for a while.

I think that this new mode with "dissassembler output" would be the same as the current --stdio, I'll check.

To further clarify, this wouldn't use any ncurses/slang TUI code, just plain printf with things formatted using what is used now for the TUI mode.

This way there would never be any drift amongst the output modes and we would have less work to do when implementing new stuff like this LBR case.

What do you think?

- Arnaldo

maintainer
#7 lore raw

Em Sat, Feb 24, 2018 at 09:40:02AM +0800, Jin, Yao escreveu:

Quoted 33 lines · Arnaldo Carvalho de Melo
Quoted 20 lines

Em Fri, Feb 23, 2018 at 09:25:00AM +0100, Peter Zijlstra escreveu:

Quoted 7 lines · Jin Yao
Quoted 5 lines

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the LBR data.

perf record -b ...
perf annotate function

Quoted 7 lines
Quoted 1 line

It should show IPC/cycle, but it doesn't.

There is far more than IPC/cycle for the LBR data, so this Changelog is misleading.

Also, I think that this patch goes the wrong way, we should reduce the divergence of the various modes, not make it worse.

Right, Peter, what would you think if I made --stdio use the same routines used to format the TUI, i.e. stdio would be equal to the TUI modulo de navigation/jump arrows, etc.

Quoted 9 lines

We would have switches to provide the TUI output options that make sense for non-interactive mode, like:

   J   Toggle showing number of jump sources on targets
   o   Toggle disassembler output/simplified view
   s   Toggle source code view
   t   Circulate percent, total period, samples view
   k   Toggle line numbers
Quoted 6 lines

Hi Arnaldo, looks your idea is very similar as my idea. In my understanding, for example, we may provide switch to tui routine like annotate_browser__write() and use fprintf() to replace ui_browser__printf()/ui_browser_write__xxx() if switch is on for stdio.

Is that your idea?

Yes, right now the TUI simply uses foo__scnprintf() routines to then pass formatted buffers to the TUI routines, we would then just pass them to plain fprintf(sdtout).

Quoted 2 lines

For this approach, I think, the benefit is we can reuse most of existing code but the disadvantage is we have to mix tui and stdio up.

- Arnaldo

author
#8 lore raw

PATCH v2 1/1 perf annotate: Support to display the IPC/Cycle in tui mode

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the IPC/Cycle even if branch info is recorded in perf data file.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

This patch lets perf annotate support the displaying of IPC/Cycle if branch info is in perf data.

For example,

perf annotate compute_flag

Percent│ IPC Cycle
       │
       │
       │                Disassembly of section .text:
       │
       │                0000000000400640 <compute_flag>:
       │                compute_flag():
       │                volatile int count;
       │                static unsigned int s_randseed;
       │
       │                __attribute__((noinline))
       │                int compute_flag()
       │                {
 22.96 │1.18   584        sub    $0x8,%rsp
       │                        int i;
       │
       │                        i = rand() % 2;
 23.02 │1.18     1      → callq  rand@plt
       │
       │                        return i;
 27.05 │3.37              mov    %eax,%edx
       │                }
       │3.37              add    $0x8,%rsp
       │                {
       │                        int i;
       │
       │                        i = rand() % 2;
       │
       │                        return i;
       │3.37              shr    $0x1f,%edx
       │3.37              add    %edx,%eax
       │3.37              and    $0x1,%eax
       │3.37              sub    %edx,%eax
       │                }
 26.97 │3.37     2      ← retq

Note that, this patch only supports tui mode. For stdio, now it just keeps original behavior. Will support it in a follow-up patch.

perf annotate compute_flag --stdio

Percent | Source code & Disassembly of div for cycles:ppp (7993 samples)
------------------------------------------------------------------------------

         :
         :
         :
         :            Disassembly of section .text:
         :
         :            0000000000400640 <compute_flag>:
         :            compute_flag():
         :            volatile int count;
         :            static unsigned int s_randseed;
         :
         :            __attribute__((noinline))
         :            int compute_flag()
         :            {
    0.29 :   400640:       sub    $0x8,%rsp     # +100.00%
         :                    int i;
         :
         :                    i = rand() % 2;
   42.93 :   400644:       callq  400490 <rand@plt>     # -100.00% (p:100.00%)
         :
         :                    return i;
    0.10 :   400649:       mov    %eax,%edx     # +100.00%
         :            }
    0.94 :   40064b:       add    $0x8,%rsp
         :            {
         :                    int i;
         :
         :                    i = rand() % 2;
         :
         :                    return i;
   27.02 :   40064f:       shr    $0x1f,%edx
    0.15 :   400652:       add    %edx,%eax
    1.24 :   400654:       and    $0x1,%eax
    2.08 :   400657:       sub    %edx,%eax
         :            }

25.26 : 400659: retq # -100.00% (p:100.00%)

Change-log:
-----------
v2:
---
No code change. Just change the patch description to make it more clear.

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/builtin-annotate.c | 88 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 6 deletions(-)
tools/perf/builtin-annotate.c +82 -6
index f15731a..ead6ae4 100644
@@ -44,6 +44,7 @@ struct perf_annotate {
 	bool	   full_paths;
 	bool	   print_line;
 	bool	   skip_missing;
+	bool	   has_br_stack;
 	const char *sym_hist_filter;
 	const char *cpu_list;
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);

@@ -146,16 +147,73 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
 	free(bi);
 }
 
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+				      struct addr_location *al __maybe_unused,
+				      bool single __maybe_unused,
+				      void *arg __maybe_unused)
+{
+	struct hist_entry *he = iter->he;
+	struct branch_info *bi;
+	struct perf_sample *sample = iter->sample;
+	struct perf_evsel *evsel = iter->evsel;
+	int err;
+
+	hist__account_cycles(sample->branch_stack, al, sample, false);
+
+	bi = he->branch_info;
+	err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx);
+
+	if (err)
+		goto out;
+
+	err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx);
+
+out:
+	return err;
+}
+
+static int process_branch_callback(struct perf_evsel *evsel,
+				   struct perf_sample *sample,
+				   struct addr_location *al __maybe_unused,
+				   struct perf_annotate *ann,
+				   struct machine *machine)
+{
+	struct hist_entry_iter iter = {
+		.evsel		= evsel,
+		.sample		= sample,
+		.add_entry_cb	= hist_iter__branch_callback,
+		.hide_unresolved	= symbol_conf.hide_unresolved,
+		.ops		= &hist_iter_branch,
+	};
+
+	struct addr_location a;
+	int ret;
+
+	if (machine__resolve(machine, &a, sample) < 0)
+		return -1;
+
+	if (a.sym == NULL)
+		return 0;
+
+	if (a.map != NULL)
+		a.map->dso->hit = 1;
+
+	ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann);
+	return ret;
+}
+
 static int perf_evsel__add_sample(struct perf_evsel *evsel,
 				  struct perf_sample *sample,
 				  struct addr_location *al,
-				  struct perf_annotate *ann)
+				  struct perf_annotate *ann,
+				  struct machine *machine)
 {
 	struct hists *hists = evsel__hists(evsel);
 	struct hist_entry *he;
 	int ret;
 
-	if (ann->sym_hist_filter != NULL &&
+	if ((!ann->has_br_stack || !ui__has_annotation()) &&
+	    ann->sym_hist_filter != NULL &&
 	    (al->sym == NULL ||
 	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
 		/* We're only interested in a symbol named sym_hist_filter */

@@ -178,6 +236,9 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 	 */
 	process_branch_stack(sample->branch_stack, al, sample);
 
+	if (ann->has_br_stack && ui__has_annotation())
+		return process_branch_callback(evsel, sample, al, ann, machine);
+
 	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;

@@ -206,7 +267,8 @@ static int process_sample_event(struct perf_tool *tool,
 	if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
 		goto out_put;
 
-	if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al, ann)) {
+	if (!al.filtered &&
+	    perf_evsel__add_sample(evsel, sample, &al, ann, machine)) {
 		pr_warning("problem incrementing symbol count, "
 			   "skipping event\n");
 		ret = -1;

@@ -238,6 +300,10 @@ static void hists__find_annotations(struct hists *hists,
 		if (he->ms.sym == NULL || he->ms.map->dso->annotate_warned)
 			goto find_next;
 
+		if (ann->sym_hist_filter &&
+		    (strcmp(he->ms.sym->name, ann->sym_hist_filter) != 0))
+			goto find_next;
+
 		notes = symbol__annotation(he->ms.sym);
 		if (notes->src == NULL) {
 find_next:

@@ -269,6 +335,7 @@ static void hists__find_annotations(struct hists *hists,
 			nd = rb_next(nd);
 		} else if (use_browser == 1) {
 			key = hist_entry__tui_annotate(he, evsel, NULL);
+
 			switch (key) {
 			case -1:
 				if (!ann->skip_missing)

@@ -489,6 +556,9 @@ int cmd_annotate(int argc, const char **argv)
 	if (annotate.session == NULL)
 		return -1;
 
+	annotate.has_br_stack = perf_header__has_feat(&annotate.session->header,
+						      HEADER_BRANCH_STACK);
+
 	ret = symbol__annotation_init();
 	if (ret < 0)
 		goto out_delete;

@@ -499,9 +569,6 @@ int cmd_annotate(int argc, const char **argv)
 	if (ret < 0)
 		goto out_delete;
 
-	if (setup_sorting(NULL) < 0)
-		usage_with_options(annotate_usage, options);
-
 	if (annotate.use_stdio)
 		use_browser = 0;
 	else if (annotate.use_tui)

@@ -511,6 +578,15 @@ int cmd_annotate(int argc, const char **argv)
 
 	setup_browser(true);
 
+	if (use_browser == 1 && annotate.has_br_stack) {
+		sort__mode = SORT_MODE__BRANCH;
+		if (setup_sorting(annotate.session->evlist) < 0)
+			usage_with_options(annotate_usage, options);
+	} else {
+		if (setup_sorting(NULL) < 0)
+			usage_with_options(annotate_usage, options);
+	}
+
 	ret = __cmd_annotate(&annotate);
 
 out_delete:
Signature · 1 line
2.7.4
author
#9 lore raw

Re: [PATCH v2] perf annotate: Support to display the IPC/Cycle in tui mode

Hi,

Could this patch be accepted?

I'm working on the supporting for stdio mode. I just wish to post the patch series after this patch being merged.

Thanks
Jin Yao

Quoted 271 lines · Jin Yao

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the IPC/Cycle even if branch info is recorded in perf data file.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

This patch lets perf annotate support the displaying of IPC/Cycle if branch info is in perf data.

For example,

perf annotate compute_flag

Percent│ IPC Cycle
        │
        │
        │                Disassembly of section .text:
        │
        │                0000000000400640 <compute_flag>:
        │                compute_flag():
        │                volatile int count;
        │                static unsigned int s_randseed;
        │
        │                __attribute__((noinline))
        │                int compute_flag()
        │                {
  22.96 │1.18   584        sub    $0x8,%rsp
        │                        int i;
        │
        │                        i = rand() % 2;
  23.02 │1.18     1      → callq  rand@plt
        │
        │                        return i;
  27.05 │3.37              mov    %eax,%edx
        │                }
        │3.37              add    $0x8,%rsp
        │                {
        │                        int i;
        │
        │                        i = rand() % 2;
        │
        │                        return i;
        │3.37              shr    $0x1f,%edx
        │3.37              add    %edx,%eax
        │3.37              and    $0x1,%eax
        │3.37              sub    %edx,%eax
        │                }
  26.97 │3.37     2      ← retq

Note that, this patch only supports tui mode. For stdio, now it just keeps original behavior. Will support it in a follow-up patch.

perf annotate compute_flag --stdio

Percent | Source code & Disassembly of div for cycles:ppp (7993 samples)
------------------------------------------------------------------------------

          :
          :
          :
          :            Disassembly of section .text:
          :
          :            0000000000400640 <compute_flag>:
          :            compute_flag():
          :            volatile int count;
          :            static unsigned int s_randseed;
          :
          :            __attribute__((noinline))
          :            int compute_flag()
          :            {
     0.29 :   400640:       sub    $0x8,%rsp     # +100.00%
          :                    int i;
          :
          :                    i = rand() % 2;
    42.93 :   400644:       callq  400490 <rand@plt>     # -100.00% (p:100.00%)
          :
          :                    return i;
     0.10 :   400649:       mov    %eax,%edx     # +100.00%
          :            }
     0.94 :   40064b:       add    $0x8,%rsp
          :            {
          :                    int i;
          :
          :                    i = rand() % 2;
          :
          :                    return i;
    27.02 :   40064f:       shr    $0x1f,%edx
     0.15 :   400652:       add    %edx,%eax
     1.24 :   400654:       and    $0x1,%eax
     2.08 :   400657:       sub    %edx,%eax
          :            }
    25.26 :   400659:       retq # -100.00% (p:100.00%)

Change-log:
-----------
v2:
---
No code change. Just change the patch description to make it more clear.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-annotate.c | 88 ++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 82 insertions(+), 6 deletions(-)
tools/perf/builtin-annotate.c +82 -6
index f15731a..ead6ae4 100644
@@ -44,6 +44,7 @@ struct perf_annotate {
  	bool	   full_paths;
  	bool	   print_line;
  	bool	   skip_missing;
+	bool	   has_br_stack;
  	const char *sym_hist_filter;
  	const char *cpu_list;
  	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);

@@ -146,16 +147,73 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
  	free(bi);
  }
 
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+				      struct addr_location *al __maybe_unused,
+				      bool single __maybe_unused,
+				      void *arg __maybe_unused)
+{
+	struct hist_entry *he = iter->he;
+	struct branch_info *bi;
+	struct perf_sample *sample = iter->sample;
+	struct perf_evsel *evsel = iter->evsel;
+	int err;
+
+	hist__account_cycles(sample->branch_stack, al, sample, false);
+
+	bi = he->branch_info;
+	err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx);
+
+	if (err)
+		goto out;
+
+	err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx);
+
+out:
+	return err;
+}
+
+static int process_branch_callback(struct perf_evsel *evsel,
+				   struct perf_sample *sample,
+				   struct addr_location *al __maybe_unused,
+				   struct perf_annotate *ann,
+				   struct machine *machine)
+{
+	struct hist_entry_iter iter = {
+		.evsel		= evsel,
+		.sample		= sample,
+		.add_entry_cb	= hist_iter__branch_callback,
+		.hide_unresolved	= symbol_conf.hide_unresolved,
+		.ops		= &hist_iter_branch,
+	};
+
+	struct addr_location a;
+	int ret;
+
+	if (machine__resolve(machine, &a, sample) < 0)
+		return -1;
+
+	if (a.sym == NULL)
+		return 0;
+
+	if (a.map != NULL)
+		a.map->dso->hit = 1;
+
+	ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann);
+	return ret;
+}
+
  static int perf_evsel__add_sample(struct perf_evsel *evsel,
  				  struct perf_sample *sample,
  				  struct addr_location *al,
-				  struct perf_annotate *ann)
+				  struct perf_annotate *ann,
+				  struct machine *machine)
  {
  	struct hists *hists = evsel__hists(evsel);
  	struct hist_entry *he;
  	int ret;
 
-	if (ann->sym_hist_filter != NULL &&
+	if ((!ann->has_br_stack || !ui__has_annotation()) &&
+	    ann->sym_hist_filter != NULL &&
  	    (al->sym == NULL ||
  	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
  		/* We're only interested in a symbol named sym_hist_filter */

@@ -178,6 +236,9 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
  	 */
  	process_branch_stack(sample->branch_stack, al, sample);
 
+	if (ann->has_br_stack && ui__has_annotation())
+		return process_branch_callback(evsel, sample, al, ann, machine);
+
  	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
  	if (he == NULL)
  		return -ENOMEM;

@@ -206,7 +267,8 @@ static int process_sample_event(struct perf_tool *tool,
  	if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
  		goto out_put;
 
-	if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al, ann)) {
+	if (!al.filtered &&
+	    perf_evsel__add_sample(evsel, sample, &al, ann, machine)) {
  		pr_warning("problem incrementing symbol count, "
  			   "skipping event\n");
  		ret = -1;

@@ -238,6 +300,10 @@ static void hists__find_annotations(struct hists *hists,
  		if (he->ms.sym == NULL || he->ms.map->dso->annotate_warned)
  			goto find_next;
 
+		if (ann->sym_hist_filter &&
+		    (strcmp(he->ms.sym->name, ann->sym_hist_filter) != 0))
+			goto find_next;
+
  		notes = symbol__annotation(he->ms.sym);
  		if (notes->src == NULL) {
  find_next:

@@ -269,6 +335,7 @@ static void hists__find_annotations(struct hists *hists,
  			nd = rb_next(nd);
  		} else if (use_browser == 1) {
  			key = hist_entry__tui_annotate(he, evsel, NULL);
+
  			switch (key) {
  			case -1:
  				if (!ann->skip_missing)

@@ -489,6 +556,9 @@ int cmd_annotate(int argc, const char **argv)
  	if (annotate.session == NULL)
  		return -1;
 
+	annotate.has_br_stack = perf_header__has_feat(&annotate.session->header,
+						      HEADER_BRANCH_STACK);
+
  	ret = symbol__annotation_init();
  	if (ret < 0)
  		goto out_delete;

@@ -499,9 +569,6 @@ int cmd_annotate(int argc, const char **argv)
  	if (ret < 0)
  		goto out_delete;
 
-	if (setup_sorting(NULL) < 0)
-		usage_with_options(annotate_usage, options);
-
  	if (annotate.use_stdio)
  		use_browser = 0;
  	else if (annotate.use_tui)

@@ -511,6 +578,15 @@ int cmd_annotate(int argc, const char **argv)
 
  	setup_browser(true);
 
+	if (use_browser == 1 && annotate.has_br_stack) {
+		sort__mode = SORT_MODE__BRANCH;
+		if (setup_sorting(annotate.session->evlist) < 0)
+			usage_with_options(annotate_usage, options);
+	} else {
+		if (setup_sorting(NULL) < 0)
+			usage_with_options(annotate_usage, options);
+	}
+
  	ret = __cmd_annotate(&annotate);
 
  out_delete:
maintainer
#10 lore raw

Re: [PATCH v2] perf annotate: Support to display the IPC/Cycle in tui mode

Em Thu, Mar 08, 2018 at 01:22:15PM +0800, Jin, Yao escreveu:

Quoted 6 lines

Hi,

Could this patch be accepted?

I'm working on the supporting for stdio mode. I just wish to post the patch series after this patch being merged.

merged, please proceed.

- Arnaldo

Quoted 265 lines

Thanks
Jin Yao

Quoted 261 lines · Jin Yao

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the IPC/Cycle even if branch info is recorded in perf data file.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

This patch lets perf annotate support the displaying of IPC/Cycle if branch info is in perf data.

For example,

perf annotate compute_flag

Percent│ IPC Cycle
        │
        │
        │                Disassembly of section .text:
        │
        │                0000000000400640 <compute_flag>:
        │                compute_flag():
        │                volatile int count;
        │                static unsigned int s_randseed;
        │
        │                __attribute__((noinline))
        │                int compute_flag()
        │                {
  22.96 │1.18   584        sub    $0x8,%rsp
        │                        int i;
        │
        │                        i = rand() % 2;
  23.02 │1.18     1      → callq  rand@plt
        │
        │                        return i;
  27.05 │3.37              mov    %eax,%edx
        │                }
        │3.37              add    $0x8,%rsp
        │                {
        │                        int i;
        │
        │                        i = rand() % 2;
        │
        │                        return i;
        │3.37              shr    $0x1f,%edx
        │3.37              add    %edx,%eax
        │3.37              and    $0x1,%eax
        │3.37              sub    %edx,%eax
        │                }
  26.97 │3.37     2      ← retq

Note that, this patch only supports tui mode. For stdio, now it just keeps original behavior. Will support it in a follow-up patch.

perf annotate compute_flag --stdio

Percent | Source code & Disassembly of div for cycles:ppp (7993 samples)
------------------------------------------------------------------------------

          :
          :
          :
          :            Disassembly of section .text:
          :
          :            0000000000400640 <compute_flag>:
          :            compute_flag():
          :            volatile int count;
          :            static unsigned int s_randseed;
          :
          :            __attribute__((noinline))
          :            int compute_flag()
          :            {
     0.29 :   400640:       sub    $0x8,%rsp     # +100.00%
          :                    int i;
          :
          :                    i = rand() % 2;
    42.93 :   400644:       callq  400490 <rand@plt>     # -100.00% (p:100.00%)
          :
          :                    return i;
     0.10 :   400649:       mov    %eax,%edx     # +100.00%
          :            }
     0.94 :   40064b:       add    $0x8,%rsp
          :            {
          :                    int i;
          :
          :                    i = rand() % 2;
          :
          :                    return i;
    27.02 :   40064f:       shr    $0x1f,%edx
     0.15 :   400652:       add    %edx,%eax
     1.24 :   400654:       and    $0x1,%eax
     2.08 :   400657:       sub    %edx,%eax
          :            }
    25.26 :   400659:       retq # -100.00% (p:100.00%)

Change-log:
-----------
v2:
---
No code change. Just change the patch description to make it more clear.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-annotate.c | 88 ++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 82 insertions(+), 6 deletions(-)
tools/perf/builtin-annotate.c +82 -6
index f15731a..ead6ae4 100644
@@ -44,6 +44,7 @@ struct perf_annotate {
  	bool	   full_paths;
  	bool	   print_line;
  	bool	   skip_missing;
+	bool	   has_br_stack;
  	const char *sym_hist_filter;
  	const char *cpu_list;
  	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);

@@ -146,16 +147,73 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
  	free(bi);
  }
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+				      struct addr_location *al __maybe_unused,
+				      bool single __maybe_unused,
+				      void *arg __maybe_unused)
+{
+	struct hist_entry *he = iter->he;
+	struct branch_info *bi;
+	struct perf_sample *sample = iter->sample;
+	struct perf_evsel *evsel = iter->evsel;
+	int err;
+
+	hist__account_cycles(sample->branch_stack, al, sample, false);
+
+	bi = he->branch_info;
+	err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx);
+
+	if (err)
+		goto out;
+
+	err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx);
+
+out:
+	return err;
+}
+
+static int process_branch_callback(struct perf_evsel *evsel,
+				   struct perf_sample *sample,
+				   struct addr_location *al __maybe_unused,
+				   struct perf_annotate *ann,
+				   struct machine *machine)
+{
+	struct hist_entry_iter iter = {
+		.evsel		= evsel,
+		.sample		= sample,
+		.add_entry_cb	= hist_iter__branch_callback,
+		.hide_unresolved	= symbol_conf.hide_unresolved,
+		.ops		= &hist_iter_branch,
+	};
+
+	struct addr_location a;
+	int ret;
+
+	if (machine__resolve(machine, &a, sample) < 0)
+		return -1;
+
+	if (a.sym == NULL)
+		return 0;
+
+	if (a.map != NULL)
+		a.map->dso->hit = 1;
+
+	ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann);
+	return ret;
+}
+
  static int perf_evsel__add_sample(struct perf_evsel *evsel,
  				  struct perf_sample *sample,
  				  struct addr_location *al,
-				  struct perf_annotate *ann)
+				  struct perf_annotate *ann,
+				  struct machine *machine)
  {
  	struct hists *hists = evsel__hists(evsel);
  	struct hist_entry *he;
  	int ret;
-	if (ann->sym_hist_filter != NULL &&
+	if ((!ann->has_br_stack || !ui__has_annotation()) &&
+	    ann->sym_hist_filter != NULL &&
  	    (al->sym == NULL ||
  	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
  		/* We're only interested in a symbol named sym_hist_filter */

@@ -178,6 +236,9 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
  	 */
  	process_branch_stack(sample->branch_stack, al, sample);
+	if (ann->has_br_stack && ui__has_annotation())
+		return process_branch_callback(evsel, sample, al, ann, machine);
+
  	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
  	if (he == NULL)
  		return -ENOMEM;

@@ -206,7 +267,8 @@ static int process_sample_event(struct perf_tool *tool,
  	if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
  		goto out_put;
-	if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al, ann)) {
+	if (!al.filtered &&
+	    perf_evsel__add_sample(evsel, sample, &al, ann, machine)) {
  		pr_warning("problem incrementing symbol count, "
  			   "skipping event\n");
  		ret = -1;

@@ -238,6 +300,10 @@ static void hists__find_annotations(struct hists *hists,
  		if (he->ms.sym == NULL || he->ms.map->dso->annotate_warned)
  			goto find_next;
+		if (ann->sym_hist_filter &&
+		    (strcmp(he->ms.sym->name, ann->sym_hist_filter) != 0))
+			goto find_next;
+
  		notes = symbol__annotation(he->ms.sym);
  		if (notes->src == NULL) {
  find_next:

@@ -269,6 +335,7 @@ static void hists__find_annotations(struct hists *hists,
  			nd = rb_next(nd);
  		} else if (use_browser == 1) {
  			key = hist_entry__tui_annotate(he, evsel, NULL);
+
  			switch (key) {
  			case -1:
  				if (!ann->skip_missing)

@@ -489,6 +556,9 @@ int cmd_annotate(int argc, const char **argv)
  	if (annotate.session == NULL)
  		return -1;
+	annotate.has_br_stack = perf_header__has_feat(&annotate.session->header,
+						      HEADER_BRANCH_STACK);
+
  	ret = symbol__annotation_init();
  	if (ret < 0)
  		goto out_delete;

@@ -499,9 +569,6 @@ int cmd_annotate(int argc, const char **argv)
  	if (ret < 0)
  		goto out_delete;
-	if (setup_sorting(NULL) < 0)
-		usage_with_options(annotate_usage, options);
-
  	if (annotate.use_stdio)
  		use_browser = 0;
  	else if (annotate.use_tui)

@@ -511,6 +578,15 @@ int cmd_annotate(int argc, const char **argv)
  	setup_browser(true);
+	if (use_browser == 1 && annotate.has_br_stack) {
+		sort__mode = SORT_MODE__BRANCH;
+		if (setup_sorting(annotate.session->evlist) < 0)
+			usage_with_options(annotate_usage, options);
+	} else {
+		if (setup_sorting(NULL) < 0)
+			usage_with_options(annotate_usage, options);
+	}
+
  	ret = __cmd_annotate(&annotate);
  out_delete:
merge bot
#11 lore raw

[tip:perf/core] perf annotate: Support to display the IPC/Cycle in TUI mode

In reply to Jin Yao
perf annotate: Support to display the IPC/Cycle in TUI mode
Commit-ID:  bb848c14f80d93059cb10b1e1446cc6823d77142
Gitweb:     https://git.kernel.org/tip/bb848c14f80d93059cb10b1e1446cc6823d77142
Author:     Jin Yao <[email protected]>
AuthorDate: Tue, 27 Feb 2018 17:38:47 +0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:52 -0300

perf annotate: Support to display the IPC/Cycle in TUI mode

Unlike the perf report interactive annotate mode, the perf annotate doesn't display the IPC/Cycle even if branch info is recorded in perf data file.

perf record -b ...
perf annotate function

It should show IPC/cycle, but it doesn't.

This patch lets perf annotate support the displaying of IPC/Cycle if branch info is in perf data.

For example,

perf annotate compute_flag

  Percent│ IPC Cycle
         │
         │
         │                Disassembly of section .text:
         │
         │                0000000000400640 <compute_flag>:
         │                compute_flag():
         │                volatile int count;
         │                static unsigned int s_randseed;
         │
         │                __attribute__((noinline))
         │                int compute_flag()
         │                {
   22.96 │1.18   584        sub    $0x8,%rsp
         │                        int i;
         │
         │                        i = rand() % 2;
   23.02 │1.18     1      → callq  rand@plt
         │
         │                        return i;
   27.05 │3.37              mov    %eax,%edx
         │                }
         │3.37              add    $0x8,%rsp
         │                {
         │                        int i;
         │
         │                        i = rand() % 2;
         │
         │                        return i;
         │3.37              shr    $0x1f,%edx
         │3.37              add    %edx,%eax
         │3.37              and    $0x1,%eax
         │3.37              sub    %edx,%eax
         │                }
   26.97 │3.37     2      ← retq

Note that, this patch only supports TUI mode. For stdio, now it just keeps original behavior. Will support it in a follow-up patch.

  $ perf annotate compute_flag --stdio
   Percent |      Source code & Disassembly of div for cycles:ppp (7993 samples)
  ------------------------------------------------------------------------------
           :
           :
           :
           :            Disassembly of section .text:
           :
           :            0000000000400640 <compute_flag>:
           :            compute_flag():
           :            volatile int count;
           :            static unsigned int s_randseed;
           :
           :            __attribute__((noinline))
           :            int compute_flag()
           :            {
      0.29 :   400640:       sub    $0x8,%rsp     # +100.00%
           :                    int i;
           :
           :                    i = rand() % 2;
     42.93 :   400644:       callq  400490 <rand@plt>     # -100.00% (p:100.00%)
           :
           :                    return i;
      0.10 :   400649:       mov    %eax,%edx     # +100.00%
           :            }
      0.94 :   40064b:       add    $0x8,%rsp
           :            {
           :                    int i;
           :
           :                    i = rand() % 2;
           :
           :                    return i;
     27.02 :   40064f:       shr    $0x1f,%edx
      0.15 :   400652:       add    %edx,%eax
      1.24 :   400654:       and    $0x1,%eax
      2.08 :   400657:       sub    %edx,%eax
           :            }
     25.26 :   400659:       retq # -100.00% (p:100.00%)
Signed-off-by Jin Yao <[email protected]>
Acked-by Andi Kleen <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Cc Alexander Shishkin <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/builtin-annotate.c | 88 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 6 deletions(-)
tools/perf/builtin-annotate.c +82 -6
index f15731a3d438..ead6ae4549e5 100644
@@ -44,6 +44,7 @@ struct perf_annotate {
 	bool	   full_paths;
 	bool	   print_line;
 	bool	   skip_missing;
+	bool	   has_br_stack;
 	const char *sym_hist_filter;
 	const char *cpu_list;
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);

@@ -146,16 +147,73 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
 	free(bi);
 }
 
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+				      struct addr_location *al __maybe_unused,
+				      bool single __maybe_unused,
+				      void *arg __maybe_unused)
+{
+	struct hist_entry *he = iter->he;
+	struct branch_info *bi;
+	struct perf_sample *sample = iter->sample;
+	struct perf_evsel *evsel = iter->evsel;
+	int err;
+
+	hist__account_cycles(sample->branch_stack, al, sample, false);
+
+	bi = he->branch_info;
+	err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx);
+
+	if (err)
+		goto out;
+
+	err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx);
+
+out:
+	return err;
+}
+
+static int process_branch_callback(struct perf_evsel *evsel,
+				   struct perf_sample *sample,
+				   struct addr_location *al __maybe_unused,
+				   struct perf_annotate *ann,
+				   struct machine *machine)
+{
+	struct hist_entry_iter iter = {
+		.evsel		= evsel,
+		.sample		= sample,
+		.add_entry_cb	= hist_iter__branch_callback,
+		.hide_unresolved	= symbol_conf.hide_unresolved,
+		.ops		= &hist_iter_branch,
+	};
+
+	struct addr_location a;
+	int ret;
+
+	if (machine__resolve(machine, &a, sample) < 0)
+		return -1;
+
+	if (a.sym == NULL)
+		return 0;
+
+	if (a.map != NULL)
+		a.map->dso->hit = 1;
+
+	ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann);
+	return ret;
+}
+
 static int perf_evsel__add_sample(struct perf_evsel *evsel,
 				  struct perf_sample *sample,
 				  struct addr_location *al,
-				  struct perf_annotate *ann)
+				  struct perf_annotate *ann,
+				  struct machine *machine)
 {
 	struct hists *hists = evsel__hists(evsel);
 	struct hist_entry *he;
 	int ret;
 
-	if (ann->sym_hist_filter != NULL &&
+	if ((!ann->has_br_stack || !ui__has_annotation()) &&
+	    ann->sym_hist_filter != NULL &&
 	    (al->sym == NULL ||
 	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
 		/* We're only interested in a symbol named sym_hist_filter */

@@ -178,6 +236,9 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 	 */
 	process_branch_stack(sample->branch_stack, al, sample);
 
+	if (ann->has_br_stack && ui__has_annotation())
+		return process_branch_callback(evsel, sample, al, ann, machine);
+
 	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;

@@ -206,7 +267,8 @@ static int process_sample_event(struct perf_tool *tool,
 	if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
 		goto out_put;
 
-	if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al, ann)) {
+	if (!al.filtered &&
+	    perf_evsel__add_sample(evsel, sample, &al, ann, machine)) {
 		pr_warning("problem incrementing symbol count, "
 			   "skipping event\n");
 		ret = -1;

@@ -238,6 +300,10 @@ static void hists__find_annotations(struct hists *hists,
 		if (he->ms.sym == NULL || he->ms.map->dso->annotate_warned)
 			goto find_next;
 
+		if (ann->sym_hist_filter &&
+		    (strcmp(he->ms.sym->name, ann->sym_hist_filter) != 0))
+			goto find_next;
+
 		notes = symbol__annotation(he->ms.sym);
 		if (notes->src == NULL) {
 find_next:

@@ -269,6 +335,7 @@ find_next:
 			nd = rb_next(nd);
 		} else if (use_browser == 1) {
 			key = hist_entry__tui_annotate(he, evsel, NULL);
+
 			switch (key) {
 			case -1:
 				if (!ann->skip_missing)

@@ -489,6 +556,9 @@ int cmd_annotate(int argc, const char **argv)
 	if (annotate.session == NULL)
 		return -1;
 
+	annotate.has_br_stack = perf_header__has_feat(&annotate.session->header,
+						      HEADER_BRANCH_STACK);
+
 	ret = symbol__annotation_init();
 	if (ret < 0)
 		goto out_delete;

@@ -499,9 +569,6 @@ int cmd_annotate(int argc, const char **argv)
 	if (ret < 0)
 		goto out_delete;
 
-	if (setup_sorting(NULL) < 0)
-		usage_with_options(annotate_usage, options);
-
 	if (annotate.use_stdio)
 		use_browser = 0;
 	else if (annotate.use_tui)

@@ -511,6 +578,15 @@ int cmd_annotate(int argc, const char **argv)
 
 	setup_browser(true);
 
+	if (use_browser == 1 && annotate.has_br_stack) {
+		sort__mode = SORT_MODE__BRANCH;
+		if (setup_sorting(annotate.session->evlist) < 0)
+			usage_with_options(annotate_usage, options);
+	} else {
+		if (setup_sorting(NULL) < 0)
+			usage_with_options(annotate_usage, options);
+	}
+
 	ret = __cmd_annotate(&annotate);
 
 out_delete: