lkml.org 
[lkml]   [2016]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 3/4] perf annotate: add powerpc support
From
Date
Thanks Naveen,

On Wednesday 29 June 2016 08:15 PM, Naveen N. Rao wrote:
> On 2016/06/29 04:45PM, Ravi Bangoria wrote:
>> From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>
>> Powerpc has long list of branch instructions and hardcoding them in
>> table appears to be error-prone. So, add new function to find
>> instruction instead of creating table. This function dynamically
>> create table(list of 'struct ins'), and instead of creating object
>> every time, first check if list already contain object for that
>> nemonics.
>>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> ---
>> Changes in v2:
>> - Corrected few memory leaks.
>> - Created Dynamic list for powerpc to optimize memory consumption
>>
>> tools/perf/util/annotate.c | 121 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 121 insertions(+)
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index 36a5825..812bfad 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -461,6 +461,11 @@ static struct ins instructions_arm[] = {
>> { .name = "bne", .ops = &jump_ops, },
>> };
>>
>> +struct instructions_powerpc {
>> + struct ins *ins;
>> + struct list_head list;
>> +};
>> +
>> static int ins__key_cmp(const void *name, const void *insp)
>> {
>> const struct ins *ins = insp;
>> @@ -476,6 +481,120 @@ static int ins__cmp(const void *a, const void *b)
>> return strcmp(ia->name, ib->name);
>> }
>>
>> +static int list_add__ins_powerpc(struct instructions_powerpc *head,
>> + struct ins *ins)
>> +{
>> + struct instructions_powerpc *ins_powerpc;
>> +
>> + ins_powerpc = zalloc(sizeof(struct instructions_powerpc));
>> + if (!ins_powerpc)
>> + return -1;
>> +
>> + ins_powerpc->ins = ins;
>> + list_add_tail(&(ins_powerpc->list), &(head->list));
>> +
>> + return 0;
>> +}
>> +
>> +static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head,
>> + const char *name)
>> +{
>> + struct instructions_powerpc *pos;
>> +
>> + list_for_each_entry(pos, &head->list, list) {
>> + if (!strcmp(pos->ins->name, name))
>> + return pos->ins;
>> + }
>> + return NULL;
>> +}
>> +
>> +static struct ins *ins__find_powerpc(const char *name)
>> +{
>> + int i;
>> + struct ins *ins;
>> + static struct instructions_powerpc head;
>> + static bool list_initialized;
>> +
>> + if (!list_initialized) {
>> + INIT_LIST_HEAD(&head.list);
>> + list_initialized = true;
>> + }
>> +
>> + /*
>> + * Search if we already created object of 'struct ins'
>> + * for this instruction
>> + */
>> + ins = list_search__ins_powerpc(&head, name);
>> + if (ins)
>> + return ins;
>> +
>> + ins = zalloc(sizeof(struct ins));
>> + if (!ins)
>> + return NULL;
>> +
>> + ins->name = strdup(name);
>> + if (!ins->name)
>> + goto err;
> You can move the above two inside the below if condition, so that you
> only allocate memory if needed.
>
> Or, what would be better would be to pass 'name' and the appropriate ops
> pointer to the helper above (list_add__ins_powerpc) and have that
> allocate 'struct ins' and insert into the list.

Yes I will think about this.

>> +
>> + if (name[0] == 'b') {
>> + /* branch instructions */
>> + ins->ops = &jump_ops;
>> +
>> + /*
>> + * - Few start with 'b', but aren't branch instructions.
>> + * - Let's also ignore instructions involving 'ctr' and
>> + * 'tar' since target branch addresses for those can't
>> + * be determined statically.
>> + */
>> + if (!strncmp(name, "bcd", 3) ||
>> + !strncmp(name, "brinc", 5) ||
>> + !strncmp(name, "bper", 4) ||
>> + strstr(name, "ctr") ||
>> + strstr(name, "tar"))
>> + goto err;
> You are still leaking ins->name here.

Ah!! Sorry. I missed that we are using strdup here. Will correct it.

-Ravi

\
 
 \ /
  Last update: 2016-06-29 18:01    [W:0.046 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site