From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753216AbcGHEyE (ORCPT ); Fri, 8 Jul 2016 00:54:04 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:55982 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751472AbcGHEx4 (ORCPT ); Fri, 8 Jul 2016 00:53:56 -0400 X-IBM-Helo: d03dlp02.boulder.ibm.com X-IBM-MailFrom: ravi.bangoria@linux.vnet.ibm.com Subject: Re: [PATCH v3 3/4] perf annotate: add powerpc support To: Michael Ellerman , linux-kernel@vger.kernel.org, acme@kernel.org, linuxppc-dev@lists.ozlabs.org References: <1467267262-4589-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <1467267262-4589-4-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <1467267714.7296.6.camel@ellerman.id.au> <57762D2A.9040103@linux.vnet.ibm.com> <87vb0j84fb.fsf@@concordia.ellerman.id.au> Cc: anton@ozlabs.org, ananth@in.ibm.com, dja@axtens.net, naveen.n.rao@linux.vnet.ibm.com, David.Laight@ACULAB.COM, Ravi Bangoria From: Ravi Bangoria Date: Fri, 8 Jul 2016 10:23:45 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <87vb0j84fb.fsf@@concordia.ellerman.id.au> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16070804-0012-0000-0000-0000101DA69E X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16070804-0013-0000-0000-00004376404E Message-Id: <577F31D9.4050808@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-08_02:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1607080046 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michael, On Wednesday 06 July 2016 03:38 PM, Michael Ellerman wrote: > Ravi Bangoria 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 >