Messages in this thread Patch in this message | | | From | Arnaldo Carvalho de Melo <> | Date | Tue, 2 Jul 2019 13:20:00 -0300 | Subject | Re: [PATCH v6 5/7] perf diff: Link same basic blocks among different data |
| |
Em Tue, Jul 02, 2019 at 01:17:39PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Jun 28, 2019 at 05:23:02PM +0800, Jin Yao escreveu: > > The target is to compare the performance difference (cycles > > diff) for the same basic blocks in different data files. > > > > The same basic block means same function, same start address > > and same end address. This patch finds the same basic blocks > > from different data files and link them together and resort > > by the cycles diff. > > > > v3: > > --- > > The block stuffs are maintained by new structure 'block_hist', > > so this patch is update accordingly. > > > > v2: > > --- > > Since now the basic block hists is changed to per symbol, > > the patch only links the basic block hists for the same > > symbol in different data files. > > > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > > --- > > tools/perf/builtin-diff.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 90 insertions(+) > > > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c > > index 83b8c0f..823f162 100644 > > --- a/tools/perf/builtin-diff.c > > +++ b/tools/perf/builtin-diff.c > > @@ -641,6 +641,85 @@ static int process_block_per_sym(struct hist_entry *he) > > return 0; > > } > > > > +static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b) > > +{ > > + struct block_info *bi_a = a->block_info; > > + struct block_info *bi_b = b->block_info; > > + int cmp; > > + > > + if (!bi_a->sym || !bi_b->sym) > > + return -1; > > + > > + if (bi_a->sym->name && bi_b->sym->name) { > > + cmp = strcmp(bi_a->sym->name, bi_b->sym->name); > > + if ((!cmp) && (bi_a->start == bi_b->start) && > > + (bi_a->end == bi_b->end)) { > > + return 0; > > + } > > > builtin-diff.c:658:17: error: address of array 'bi_a->sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] > if (bi_a->sym->name && bi_b->sym->name) { > ~~~~~~~~~~~^~~~ ~~ > builtin-diff.c:658:36: error: address of array 'bi_b->sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] > if (bi_a->sym->name && bi_b->sym->name) { > > Because: > > struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name) > { > size_t namelen = strlen(name) + 1; > struct symbol *sym = calloc(1, (symbol_conf.priv_size + > sizeof(*sym) + namelen)); > > > So it will be at least a strlen(sym->name) == 0, i.e. we can use it > without checking anything. > > I'm chanign it to do the cmp straight away
i.e. I added this on top of this patch:
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 823f162b0060..fafb7b3f58fb 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -650,13 +650,10 @@ static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b) if (!bi_a->sym || !bi_b->sym) return -1; - if (bi_a->sym->name && bi_b->sym->name) { - cmp = strcmp(bi_a->sym->name, bi_b->sym->name); - if ((!cmp) && (bi_a->start == bi_b->start) && - (bi_a->end == bi_b->end)) { - return 0; - } - } + cmp = strcmp(bi_a->sym->name, bi_b->sym->name); + + if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end)) + return 0; return -1; }
| |