On Thursday, May 4, 2017 10:31:37 AM CEST Jin Yao wrote:
> perf record -b -g <command>
> perf report --branch-history
> 
> This merges the LBRs with the callgraphs.
> 
> However it would be nice if it also works without callgraphs (-g)
> set in perf record, so that only the LBRs are displayed.
> But currently perf report errors in this case. For example,
> 
> perf record -b <command>
> perf report --branch-history
> 
> Error:
> Selected -g or --branch-history but no callchain data. Did
> you call 'perf record' without -g?
> 
> This patch displays the LBRs only even if callgraphs(-g) is not
> enabled in perf record.
> 
> Signed-off-by: Jin Yao <[email protected]>
> ---
>  tools/perf/builtin-report.c |  6 ++++--
>  tools/perf/util/callchain.c |  7 ++++---
>  tools/perf/util/hist.c      |  2 ++
>  tools/perf/util/machine.c   | 13 ++++++++++++-
>  4 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 22478ff..7f82369 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -259,7 +259,8 @@ static int report__setup_sample_type(struct report *rep)
> "'perf record' without -g?\n");
>                       return -EINVAL;
>               }
> -             if (symbol_conf.use_callchain) {
> +             if (symbol_conf.use_callchain &&
> +                     !symbol_conf.show_branchflag_count) {
>                       ui__error("Selected -g or --branch-history but no "
>                                 "callchain data. Did\n"
>                                 "you call 'perf record' without -g?\n");

The `--branch-history` part of this error message is now stale, no?


> @@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct
> perf_evlist *evlist,
> 
>               hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
>               hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
> -                            symbol_conf.use_callchain);
> +                            symbol_conf.use_callchain |
> +                            symbol_conf.show_branchflag_count);
>               fprintf(stdout, "\n\n");
>       }

Not sure about the coding style here, but shouldn't this be a boolean or 
operator here '||' - semantically? Functionality wise it will be the same, I 
guess?

> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 81fc29a..08d3abf 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample
> *sample, struct perf_evsel *evsel, struct addr_location *al,
>                             int max_stack)
>  {
> -     if (sample->callchain == NULL)
> +     if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
>               return 0;
> 
>       if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
> -         perf_hpp_list.parent) {
> +         perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
>               return thread__resolve_callchain(al->thread, cursor, evsel, 
> sample,
>                                                parent, al, max_stack);
>       }
> @@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample
> *sample,
> 
>  int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample
> *sample) {
> -     if (!symbol_conf.use_callchain || sample->callchain == NULL)
> +     if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
> +             !symbol_conf.show_branchflag_count)
>               return 0;
>       return callchain_append(he->callchain, &callchain_cursor, 
> sample->period);
> }
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index cf0186a..8b045a5 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel
> *evsel, struct ui_progress *pro else
>               use_callchain = symbol_conf.use_callchain;
> 
> +     use_callchain |= symbol_conf.show_branchflag_count;
> +
>       output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
>  }
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index a98f55a..792ddcd 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, {
>       struct branch_stack *branch = sample->branch_stack;
>       struct ip_callchain *chain = sample->callchain;
> -     int chain_nr = chain->nr;
> +     int chain_nr = 0;
>       u8 cpumode = PERF_RECORD_MISC_USER;
>       int i, j, err, nr_entries;
>       int skip_idx = -1;
>       int first_call = 0;
>       int nr_loop_iter;
> 
> +     if (chain)
> +             chain_nr = chain->nr;
> +
>       if (perf_evsel__has_branch_callstack(evsel)) {
>               err = resolve_lbr_callchain_sample(thread, cursor, sample, 
> parent,
>                                                  root_al, max_stack);
> @@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, for (i = 0; i < nr; i++) {
>                       if (callchain_param.order == ORDER_CALLEE) {
>                               be[i] = branch->entries[i];
> +
> +                             if (chain == NULL)
> +                                     continue;
> +
>                               /*
>                                * Check for overlap into the callchain.
>                                * The return address is one off compared to
> @@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, if (err)
>                               return err;
>               }
> +
> +             if (chain_nr == 0)
> +                     return 0;
> +
>               chain_nr -= nr;
>       }


-- 
Milian Wolff | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

Reply via email to