| Message ID | 1467267262-4589-4-git-send-email-ravi.bangoria@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 36a5825..b87eac7 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) ... > + > +static struct ins *ins__find_powerpc(const char *name) > +{ > + int i; > + struct ins *ins; > + struct ins_ops *ops; > + static struct instructions_powerpc head; > + static bool list_initialized; > + > + /* > + * - Interested only if instruction starts with 'b'. > + * - 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 (name[0] != 'b' || > + !strncmp(name, "bcd", 3) || > + !strncmp(name, "brinc", 5) || > + !strncmp(name, "bper", 4) || > + strstr(name, "ctr") || > + strstr(name, "tar")) > + return NULL; It would be good if 'bctr' was at least recognised as a branch, even if we can't determine the target. They are very common. It doesn't look like we have the opcode handy here? Could we get it somehow? That would make this a *lot* more robust. cheers
Thanks Michael for your suggestion. On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: > On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index 36a5825..b87eac7 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) > ... >> + >> +static struct ins *ins__find_powerpc(const char *name) >> +{ >> + int i; >> + struct ins *ins; >> + struct ins_ops *ops; >> + static struct instructions_powerpc head; >> + static bool list_initialized; >> + >> + /* >> + * - Interested only if instruction starts with 'b'. >> + * - 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 (name[0] != 'b' || >> + !strncmp(name, "bcd", 3) || >> + !strncmp(name, "brinc", 5) || >> + !strncmp(name, "bper", 4) || >> + strstr(name, "ctr") || >> + strstr(name, "tar")) >> + return NULL; > It would be good if 'bctr' was at least recognised as a branch, even if we > can't determine the target. They are very common. We can not show arrow for this since we don't know the target location. can you please suggest how you intends perf to display bctr? bctr can be classified into two variants -- 'bctr' and 'bctrl'. 'bctr' will be considered as jump instruction but jump__parse() won't be able to find any target location and hence it will set target to UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This looks misleading. bctrl will be considered as call instruction but call_parse() won't be able to find any target function and hence it won't show any navigation arrow for this instruction. Which is same as filter it beforehand. > It doesn't look like we have the opcode handy here? Could we get it somehow? > That would make this a *lot* more robust. objdump prints machine code, but I don't know how difficult that would be to parse to get opcode. -Ravi > cheers >
On Fri, 2016-07-01 at 14:13 +0530, Ravi Bangoria wrote: > Thanks Michael for your suggestion. > > On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: > > > > On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: > > > > > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > > > index 36a5825..b87eac7 100644 > > > --- a/tools/perf/util/annotate.c > > > +++ b/tools/perf/util/annotate.c > > > @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) > > ... > > > > > > + > > > +static struct ins *ins__find_powerpc(const char *name) > > > +{ > > > + int i; > > > + struct ins *ins; > > > + struct ins_ops *ops; > > > + static struct instructions_powerpc head; > > > + static bool list_initialized; > > > + > > > + /* > > > + * - Interested only if instruction starts with 'b'. > > > + * - 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 (name[0] != 'b' || > > > + !strncmp(name, "bcd", 3) || > > > + !strncmp(name, "brinc", 5) || > > > + !strncmp(name, "bper", 4) || > > > + strstr(name, "ctr") || > > > + strstr(name, "tar")) > > > + return NULL; > > It would be good if 'bctr' was at least recognised as a branch, even if we > > can't determine the target. They are very common. > We can not show arrow for this since we don't know the target location. > can you please suggest how you intends perf to display bctr? > > bctr can be classified into two variants -- 'bctr' and 'bctrl'. > > 'bctr' will be considered as jump instruction but jump__parse() won't > be able to find any target location and hence it will set target to > UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This > looks misleading. > > bctrl will be considered as call instruction but call_parse() won't > be able to find any target function and hence it won't show any > navigation arrow for this instruction. Which is same as filter it > beforehand. > The target location and function are in the counter. Can't we add this to instruction ops? Is it a major change to add it? Balbir Singh.
Hi Balbir, On Friday 01 July 2016 06:18 PM, Balbir Singh wrote: > On Fri, 2016-07-01 at 14:13 +0530, Ravi Bangoria wrote: >> Thanks Michael for your suggestion. >> >> On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: >>> >>> On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: >>>> >>>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>>> index 36a5825..b87eac7 100644 >>>> --- a/tools/perf/util/annotate.c >>>> +++ b/tools/perf/util/annotate.c >>>> @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) >>> ... >>>> >>>> + >>>> +static struct ins *ins__find_powerpc(const char *name) >>>> +{ >>>> + int i; >>>> + struct ins *ins; >>>> + struct ins_ops *ops; >>>> + static struct instructions_powerpc head; >>>> + static bool list_initialized; >>>> + >>>> + /* >>>> + * - Interested only if instruction starts with 'b'. >>>> + * - 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 (name[0] != 'b' || >>>> + !strncmp(name, "bcd", 3) || >>>> + !strncmp(name, "brinc", 5) || >>>> + !strncmp(name, "bper", 4) || >>>> + strstr(name, "ctr") || >>>> + strstr(name, "tar")) >>>> + return NULL; >>> It would be good if 'bctr' was at least recognised as a branch, even if we >>> can't determine the target. They are very common. >> We can not show arrow for this since we don't know the target location. >> can you please suggest how you intends perf to display bctr? >> >> bctr can be classified into two variants -- 'bctr' and 'bctrl'. >> >> 'bctr' will be considered as jump instruction but jump__parse() won't >> be able to find any target location and hence it will set target to >> UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This >> looks misleading. >> >> bctrl will be considered as call instruction but call_parse() won't >> be able to find any target function and hence it won't show any >> navigation arrow for this instruction. Which is same as filter it >> beforehand. >> > The target location and function are in the counter. Can't we add > this to instruction ops? Is it a major change to add it? Of course we can add it. What I mean is we can not determine target location statically by parsing objdump output. For example, consider snippet: objdump output: c000000000143848: lwarx r8,0,r10 c00000000014384c: addic r8,r8,1 c000000000143850: stwcx. r8,0,r10 c000000000143854: bne- c000000000143848 <.rcu_idle_exit+0x58> corresponding perf annotate output: 58: lwarx r8,0,r10 addic r8,r8,1 stwcx. r8,0,r10 bne- 58 tui will show up arrow before 'bne- 58' instruction, that indicate it as a jump instruction. When we focus on 'bne- 58' instruction, arrow will span from that instruction to instruction with 58th offset( lwarx ). By pressing Enter, it will jump focus to the target. In case of 'bctr', we can not determine target location statically and hence we can not provide any navigation options. Same for 'bctrl' as well. Please correct me if I misunderstood anything. -Ravi > > Balbir Singh. >
Hi Michael, On Friday 01 July 2016 02:13 PM, Ravi Bangoria wrote: > Thanks Michael for your suggestion. > > On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: >> On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: >>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>> index 36a5825..b87eac7 100644 >>> --- a/tools/perf/util/annotate.c >>> +++ b/tools/perf/util/annotate.c >>> @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) >> ... >>> + >>> +static struct ins *ins__find_powerpc(const char *name) >>> +{ >>> + int i; >>> + struct ins *ins; >>> + struct ins_ops *ops; >>> + static struct instructions_powerpc head; >>> + static bool list_initialized; >>> + >>> + /* >>> + * - Interested only if instruction starts with 'b'. >>> + * - 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 (name[0] != 'b' || >>> + !strncmp(name, "bcd", 3) || >>> + !strncmp(name, "brinc", 5) || >>> + !strncmp(name, "bper", 4) || >>> + strstr(name, "ctr") || >>> + strstr(name, "tar")) >>> + return NULL; >> It would be good if 'bctr' was at least recognised as a branch, even >> if we >> can't determine the target. They are very common. > > We can not show arrow for this since we don't know the target location. > can you please suggest how you intends perf to display bctr? > > bctr can be classified into two variants -- 'bctr' and 'bctrl'. > > 'bctr' will be considered as jump instruction but jump__parse() won't > be able to find any target location and hence it will set target to > UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This > looks misleading. > > bctrl will be considered as call instruction but call_parse() won't > be able to find any target function and hence it won't show any > navigation arrow for this instruction. Which is same as filter it > beforehand. > >> It doesn't look like we have the opcode handy here? Could we get it >> somehow? >> That would make this a *lot* more robust. > > objdump prints machine code, but I don't know how difficult that would > be to parse to get opcode. Perf uses --no-show-raw with objdump and hence objdump output does not show opcodes. So change in current objdump output may requires changes in current parsing logic. Additionally I need to change tui as well to show opcodes. This looks quite more work. And this patchset is about enabling annotate for cross arch. So if you really need opcode with perf anotate, can we do it separately? Please let me know your thoughts. -Ravi > > -Ravi > >> cheers >> >
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: > On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: >> On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: >>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>> index 36a5825..b87eac7 100644 >>> --- a/tools/perf/util/annotate.c >>> +++ b/tools/perf/util/annotate.c >>> @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) >> ... >>> + >>> +static struct ins *ins__find_powerpc(const char *name) >>> +{ >>> + int i; >>> + struct ins *ins; >>> + struct ins_ops *ops; >>> + static struct instructions_powerpc head; >>> + static bool list_initialized; >>> + >>> + /* >>> + * - Interested only if instruction starts with 'b'. >>> + * - 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 (name[0] != 'b' || >>> + !strncmp(name, "bcd", 3) || >>> + !strncmp(name, "brinc", 5) || >>> + !strncmp(name, "bper", 4) || >>> + strstr(name, "ctr") || >>> + strstr(name, "tar")) >>> + return NULL; >> It would be good if 'bctr' was at least recognised as a branch, even if we >> can't determine the target. They are very common. > > We can not show arrow for this since we don't know the target location. > can you please suggest how you intends perf to display bctr? Yeah I understand you can't show an arrow. I guess it could just be an unterminated arrow? But I'm not sure if that's easy to do with the way the UI is constructed. eg. something like: ld r12,0(r12) mtctr r12 bctrl ------------------> ld r3,-32704(r2) But that's just an idea. > bctr can be classified into two variants -- 'bctr' and 'bctrl'. > > 'bctr' will be considered as jump instruction but jump__parse() won't > be able to find any target location and hence it will set target to > UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This > looks misleading. Agreed. > bctrl will be considered as call instruction but call_parse() won't > be able to find any target function and hence it won't show any > navigation arrow for this instruction. Which is same as filter it > beforehand. OK. Maybe what I'm asking for is an enhancement and can be done later. >> It doesn't look like we have the opcode handy here? Could we get it somehow? >> That would make this a *lot* more robust. > > objdump prints machine code, but I don't know how difficult that would > be to parse to get opcode. Normal objdump -d output includes the opcode, eg: c00000000000886c: 2c 2c 00 00 cmpdi r12,0 ^^^^^^^^^^^ The only thing you need to know is the endian and you can reconstruct the raw instruction. Then you can just decode the opcode, see how we do it in the kernel with eg. instr_is_relative_branch(). cheers
Hi Michael, On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: > >> On Thursday 30 June 2016 11:51 AM, Michael Ellerman wrote: >>> On Thu, 2016-06-30 at 11:44 +0530, Ravi Bangoria wrote: >>>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>>> index 36a5825..b87eac7 100644 >>>> --- a/tools/perf/util/annotate.c >>>> +++ b/tools/perf/util/annotate.c >>>> @@ -476,6 +481,125 @@ static int ins__cmp(const void *a, const void *b) >>> ... >>>> + >>>> +static struct ins *ins__find_powerpc(const char *name) >>>> +{ >>>> + int i; >>>> + struct ins *ins; >>>> + struct ins_ops *ops; >>>> + static struct instructions_powerpc head; >>>> + static bool list_initialized; >>>> + >>>> + /* >>>> + * - Interested only if instruction starts with 'b'. >>>> + * - 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 (name[0] != 'b' || >>>> + !strncmp(name, "bcd", 3) || >>>> + !strncmp(name, "brinc", 5) || >>>> + !strncmp(name, "bper", 4) || >>>> + strstr(name, "ctr") || >>>> + strstr(name, "tar")) >>>> + return NULL; >>> It would be good if 'bctr' was at least recognised as a branch, even if we >>> can't determine the target. They are very common. >> We can not show arrow for this since we don't know the target location. >> can you please suggest how you intends perf to display bctr? > Yeah I understand you can't show an arrow. > > I guess it could just be an unterminated arrow? But I'm not sure if > that's easy to do with the way the UI is constructed. eg. something > like: > > ld r12,0(r12) > mtctr r12 > bctrl ------------------> > ld r3,-32704(r2) > > But that's just an idea. I've sent v4 which enables annotate for bctr' instructions. for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show right arrow(indicate call). But no navigation options will be provided. By pressing Enter key on that, message will be shown that like "Invalid target" Please review it. >> bctr can be classified into two variants -- 'bctr' and 'bctrl'. >> >> 'bctr' will be considered as jump instruction but jump__parse() won't >> be able to find any target location and hence it will set target to >> UINT64_MAX which transform 'bctr' to 'bctr UINT64_MAX'. This >> looks misleading. > Agreed. > >> bctrl will be considered as call instruction but call_parse() won't >> be able to find any target function and hence it won't show any >> navigation arrow for this instruction. Which is same as filter it >> beforehand. > OK. > > Maybe what I'm asking for is an enhancement and can be done later. > >>> It doesn't look like we have the opcode handy here? Could we get it somehow? >>> That would make this a *lot* more robust. >> objdump prints machine code, but I don't know how difficult that would >> be to parse to get opcode. > Normal objdump -d output includes the opcode, eg: > > c00000000000886c: 2c 2c 00 00 cmpdi r12,0 > ^^^^^^^^^^^ > > The only thing you need to know is the endian and you can reconstruct > the raw instruction. > > Then you can just decode the opcode, see how we do it in the kernel with > eg. instr_is_relative_branch(). I'm sorry. I was thinking that you wants to show opcodes with perf annotate. But you were asking to use opcode instead of parsing instructions. This looks like rewrite parsing code. I don't know whether there is any library already available for this which we can directly use. I'm thinking about this. - Ravi > cheers >
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: > On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: > > I've sent v4 which enables annotate for bctr' instructions. > > for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show > right arrow(indicate call). But no navigation options will be provided. > By pressing Enter key on that, message will be shown that like > "Invalid target" Great thanks. >>>> It doesn't look like we have the opcode handy here? Could we get it somehow? >>>> That would make this a *lot* more robust. >>> objdump prints machine code, but I don't know how difficult that would >>> be to parse to get opcode. >> Normal objdump -d output includes the opcode, eg: >> >> c00000000000886c: 2c 2c 00 00 cmpdi r12,0 >> ^^^^^^^^^^^ >> >> The only thing you need to know is the endian and you can reconstruct >> the raw instruction. >> >> Then you can just decode the opcode, see how we do it in the kernel with >> eg. instr_is_relative_branch(). > > I'm sorry. I was thinking that you wants to show opcodes with perf > annotate. But you were asking to use opcode instead of parsing > instructions. Yeah. > This looks like rewrite parsing code. I don't know whether there is any > library already available for this which we can directly use. I'm thinking > about this. OK don't worry about it for now. We should get this merged for starters and we can always improve it later. cheers
Hi Arnaldo, On Friday 08 July 2016 02:01 PM, Michael Ellerman wrote: > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: > >> On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: >> >> I've sent v4 which enables annotate for bctr' instructions. >> >> for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show >> right arrow(indicate call). But no navigation options will be provided. >> By pressing Enter key on that, message will be shown that like >> "Invalid target" > Great thanks. I've sent v4 series. Please review it. -Ravi >>>>> It doesn't look like we have the opcode handy here? Could we get it somehow? >>>>> That would make this a *lot* more robust. >>>> objdump prints machine code, but I don't know how difficult that would >>>> be to parse to get opcode. >>> Normal objdump -d output includes the opcode, eg: >>> >>> c00000000000886c: 2c 2c 00 00 cmpdi r12,0 >>> ^^^^^^^^^^^ >>> >>> The only thing you need to know is the endian and you can reconstruct >>> the raw instruction. >>> >>> Then you can just decode the opcode, see how we do it in the kernel with >>> eg. instr_is_relative_branch(). >> I'm sorry. I was thinking that you wants to show opcodes with perf >> annotate. But you were asking to use opcode instead of parsing >> instructions. > Yeah. > >> This looks like rewrite parsing code. I don't know whether there is any >> library already available for this which we can directly use. I'm thinking >> about this. > OK don't worry about it for now. We should get this merged for starters > and we can always improve it later. > > cheers >
Em Tue, Jul 12, 2016 at 07:51:46AM +0530, Ravi Bangoria escreveu: > Hi Arnaldo, > > On Friday 08 July 2016 02:01 PM, Michael Ellerman wrote: > > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: > > > > > On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: > > > > > > I've sent v4 which enables annotate for bctr' instructions. > > > > > > for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show > > > right arrow(indicate call). But no navigation options will be provided. > > > By pressing Enter key on that, message will be shown that like > > > "Invalid target" > > Great thanks. > > I've sent v4 series. Please review it. If somebody else could do it and provide acks/reviewed by, that would help, Michael, can I get your comments as such? Thanks, - Arnaldo > -Ravi > > > > > > > It doesn't look like we have the opcode handy here? Could we get it somehow? > > > > > > That would make this a *lot* more robust. > > > > > objdump prints machine code, but I don't know how difficult that would > > > > > be to parse to get opcode. > > > > Normal objdump -d output includes the opcode, eg: > > > > > > > > c00000000000886c: 2c 2c 00 00 cmpdi r12,0 > > > > ^^^^^^^^^^^ > > > > > > > > The only thing you need to know is the endian and you can reconstruct > > > > the raw instruction. > > > > > > > > Then you can just decode the opcode, see how we do it in the kernel with > > > > eg. instr_is_relative_branch(). > > > I'm sorry. I was thinking that you wants to show opcodes with perf > > > annotate. But you were asking to use opcode instead of parsing > > > instructions. > > Yeah. > > > > > This looks like rewrite parsing code. I don't know whether there is any > > > library already available for this which we can directly use. I'm thinking > > > about this. > > OK don't worry about it for now. We should get this merged for starters > > and we can always improve it later. > > > > cheers > >
Arnaldo Carvalho de Melo <acme@kernel.org> writes: > Em Tue, Jul 12, 2016 at 07:51:46AM +0530, Ravi Bangoria escreveu: >> Hi Arnaldo, >> >> On Friday 08 July 2016 02:01 PM, Michael Ellerman wrote: >> > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: >> > >> > > On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: >> > > >> > > I've sent v4 which enables annotate for bctr' instructions. >> > > >> > > for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show >> > > right arrow(indicate call). But no navigation options will be provided. >> > > By pressing Enter key on that, message will be shown that like >> > > "Invalid target" >> > Great thanks. >> >> I've sent v4 series. Please review it. > > If somebody else could do it and provide acks/reviewed by, that would > help, > > Michael, can I get your comments as such? It looks OK to me. But I don't know the code really, and I haven't had time to test it personally. Ravi, have you tested on a big endian machine? cheers
On Wednesday 13 July 2016 01:09 PM, Michael Ellerman wrote: > Arnaldo Carvalho de Melo <acme@kernel.org> writes: > >> Em Tue, Jul 12, 2016 at 07:51:46AM +0530, Ravi Bangoria escreveu: >>> Hi Arnaldo, >>> >>> On Friday 08 July 2016 02:01 PM, Michael Ellerman wrote: >>>> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> writes: >>>> >>>>> On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: >>>>> >>>>> I've sent v4 which enables annotate for bctr' instructions. >>>>> >>>>> for 'bctr', it will show down arrow(indicate jump) and 'bctrl' will show >>>>> right arrow(indicate call). But no navigation options will be provided. >>>>> By pressing Enter key on that, message will be shown that like >>>>> "Invalid target" >>>> Great thanks. >>> I've sent v4 series. Please review it. >> If somebody else could do it and provide acks/reviewed by, that would >> help, >> >> Michael, can I get your comments as such? > It looks OK to me. But I don't know the code really, and I haven't had > time to test it personally. > > Ravi, have you tested on a big endian machine? Yes Michael, I've tested annotate on BE and LE both. -Ravi > > cheers >
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 36a5825..b87eac7 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,125 @@ static int ins__cmp(const void *a, const void *b) return strcmp(ia->name, ib->name); } +static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, + const char *name, struct ins_ops *ops) +{ + struct instructions_powerpc *ins_powerpc; + struct ins *ins; + + ins = zalloc(sizeof(struct ins)); + if (!ins) + return NULL; + + ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); + if (!ins_powerpc) + goto out_free_ins; + + ins->name = strdup(name); + if (!ins->name) + goto out_free_ins_power; + + ins->ops = ops; + ins_powerpc->ins = ins; + list_add_tail(&(ins_powerpc->list), &(head->list)); + + return ins; + +out_free_ins_power: + zfree(&ins_powerpc); +out_free_ins: + zfree(&ins); + return NULL; +} + +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; + struct ins_ops *ops; + static struct instructions_powerpc head; + static bool list_initialized; + + /* + * - Interested only if instruction starts with 'b'. + * - 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 (name[0] != 'b' || + !strncmp(name, "bcd", 3) || + !strncmp(name, "brinc", 5) || + !strncmp(name, "bper", 4) || + strstr(name, "ctr") || + strstr(name, "tar")) + return NULL; + + if (!list_initialized) { + INIT_LIST_HEAD(&head.list); + list_initialized = true; + } + + /* + * Return if we already have object of 'struct ins' for this + * instruction + */ + ins = list_search__ins_powerpc(&head, name); + if (ins) + return ins; + + ops = &jump_ops; + + i = strlen(name) - 1; + if (i < 0) + return NULL; + + /* ignore optional hints at the end of the instructions */ + if (name[i] == '+' || name[i] == '-') + i--; + + if (name[i] == 'l' || (name[i] == 'a' && name[i-1] == 'l')) { + /* + * if the instruction ends up with 'l' or 'la', then + * those are considered 'calls' since they update LR. + * ... except for 'bnl' which is branch if not less than + * and the absolute form of the same. + */ + if (strcmp(name, "bnl") && strcmp(name, "bnl+") && + strcmp(name, "bnl-") && strcmp(name, "bnla") && + strcmp(name, "bnla+") && strcmp(name, "bnla-")) + ops = &call_ops; + } + if (name[i] == 'r' && name[i-1] == 'l') + /* + * instructions ending with 'lr' are considered to be + * return instructions + */ + ops = &ret_ops; + + /* + * Add instruction to list so next time no need to + * allocate memory for it. + */ + ins = list_add__ins_powerpc(&head, name, ops); + if (ins) + return ins; + + return NULL; +} + static void ins__sort(struct ins *instructions, int nmemb) { qsort(instructions, nmemb, sizeof(struct ins), ins__cmp); @@ -511,6 +635,8 @@ static struct ins *ins__find(const char *name, const char *norm_arch) } else if (!strcmp(norm_arch, "arm")) { instructions = instructions_arm; nmemb = ARRAY_SIZE(instructions_arm); + } else if (!strcmp(norm_arch, "powerpc")) { + return ins__find_powerpc(name); } else { pr_err("perf annotate not supported by %s arch\n", norm_arch); return NULL;