lkml.org 
[lkml]   [2019]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 5/7] perf diff: Link same basic blocks among different data
From
Date


On 7/3/2019 12:20 AM, Arnaldo Carvalho de Melo wrote:
> 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;
> }
>

Hi Arnaldo,

Thanks so much for providing the fix. Maybe my gcc version is too old.
gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)

I will switch to gcc version 7.4.0.

Thanks
Jin Yao


\
 
 \ /
  Last update: 2019-07-03 03:08    [W:0.045 / U:5.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site