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 <[email protected]>
> > ---
> > 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;
}
