Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp8272407rwd; Tue, 20 Jun 2023 12:40:39 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6CCRflz6R+Iq0gBk5UKO7vrzHqNzCxmwvneI1qnnDPIA8hO7NcZrGStn7LxOnOO57O4PKL X-Received: by 2002:a17:903:1208:b0:1a5:150f:8558 with SMTP id l8-20020a170903120800b001a5150f8558mr14620081plh.17.1687290039381; Tue, 20 Jun 2023 12:40:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687290039; cv=none; d=google.com; s=arc-20160816; b=Tws3WN4q7ao56whRUN2CtOz4Lvg82ShhFzDWz+cxIjnadX28kSOfxyQw5FLKD7aD/5 DTmmloDUj95gHlT5pCsrxlRW0d/IKDPbMvc0MaPTnpZAwcEIl+rkXZaHZ+S/7Kswkwsm BOd7ZM7+WnpbTqV9AfiqhdhmcICTITO9+AEyTGd72PrkdB0JPx1R914xgPLumkX5t9My ogMvAk+ZOZd2yivX5mvBwV85GCA3+xeyqV8zcjnAZKVdEljBthamzzRIiUYArsSPsfq9 4VUSZnWa+qdNE17MHdwZMS1KNMeNKuOzIvSC+BE9zlH5N85ZZMBpYNlPgumDnv9Yap5z oNBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version; bh=iQRa/mz36CwdC5zTdxUK1uH73+13wMXIGMBSTlOjxD0=; b=Bii7MPL86ESi7FmiyIX17BxebwWm2PUNE+5ZP7PhRyKIjWlOpq1G7uwtysRrlPdJrj iCpkOuEkbdmNXNOnJ6HMMhbHwlBa0at0D3KCMsH9T2yu2bygrgRm2sSWLVs8fQVqWzyy r5UFYwlcbCqs49H5NrpA3FVsCEvGK4yXpU5k2y/1+Q4ML1NWP4kgxMJQoIAkOTlAj80b a3RfrgByJzWHJQ1JsedGVcdlpvIPQm7a1L1aGGEqN1LLQO0DWCIpEiiRxQ/ghEyIu7Vd tArFv49OYPN+DL25sYGnRZ/XEc4xLSOkp5WxisF76ZAyTX9EAlTReO6z2mWiFKAnabTX s0Wg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e17-20020a170902f11100b001b041114a62si2388721plb.355.2023.06.20.12.40.26; Tue, 20 Jun 2023 12:40:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230093AbjFTSml convert rfc822-to-8bit (ORCPT + 99 others); Tue, 20 Jun 2023 14:42:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37220 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229876AbjFTSmk (ORCPT ); Tue, 20 Jun 2023 14:42:40 -0400 Received: from mail-io1-f50.google.com (mail-io1-f50.google.com [209.85.166.50]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 31986DC; Tue, 20 Jun 2023 11:42:39 -0700 (PDT) Received: by mail-io1-f50.google.com with SMTP id ca18e2360f4ac-77e457319a6so72913239f.1; Tue, 20 Jun 2023 11:42:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687286558; x=1689878558; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sCtMr8ijFx2CrFh9ZGovTyfqeQbUmmEvN/2Rn9Z2qpQ=; b=Is5zqw8te+z6zLolRfjtZeG56es4WN4izEWadogetXeF0tVUgOeDJpF35BxPDifggy Yyc5Myj3wZ6UGsGeZEI/WT2Zq1EN1HkK5FiiGV8qw/TKeS7Hzhe8eARs9qpjIcsh8jEc MNh30GO2KlTI6C/b21SzJT+I9XNyfvf2/doXtHEHxXitBfgB0NvhiMWLE6plE8Xf+9t6 BBnvTXDFWSiXHvx3cGI8TyIFTPghir3GuUnsL1kJUSjjrS1TJPUie5fOW4uvkDsr4ByV rKonPPR05RYQ3QSL8mxxHVGBOQK63VjWNoKUQrTcjKAOx7/3vn1j9KsDH/Dct28GlArq Hi5g== X-Gm-Message-State: AC+VfDzZ2tvwjZWPKh2+qNAK56NjqvUBfJUDtT9BtxTOogKeKgd/eikc NNjrAirxxKeLhH8zB+PkAyjC+mvQDVK1uiD9yJrt9oei X-Received: by 2002:a6b:fd07:0:b0:777:ce47:6c5a with SMTP id c7-20020a6bfd07000000b00777ce476c5amr9165132ioi.21.1687286557979; Tue, 20 Jun 2023 11:42:37 -0700 (PDT) MIME-Version: 1.0 References: <20230620132025.105563-1-wangrui@loongson.cn> In-Reply-To: <20230620132025.105563-1-wangrui@loongson.cn> From: Namhyung Kim Date: Tue, 20 Jun 2023 11:42:26 -0700 Message-ID: Subject: Re: [PATCH] perf annotate: Fix instruction association and parsing for LoongArch To: WANG Rui Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Huacai Chen , loongarch@lists.linux.dev, Tiezhu Yang , WANG Xuerui , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, loongson-kernel@lists.loongnix.cn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Tue, Jun 20, 2023 at 6:21 AM WANG Rui wrote: > > In the perf annotate view for LoongArch, there is no arrowed line > pointing to the target from the branch instruction. This issue is > caused by incorrect instruction association and parsing. > > $ perf record alloc-6276705c94ad1398 # rust benchmark > $ perf report > > 0.28 │ ori $a1, $zero, 0x63 > │ move $a2, $zero > 10.55 │ addi.d $a3, $a2, 1(0x1) > │ sltu $a4, $a3, $s7 > 9.53 │ masknez $a4, $s7, $a4 > │ sub.d $a3, $a3, $a4 > 12.12 │ st.d $a1, $fp, 24(0x18) > │ st.d $a3, $fp, 16(0x10) > 16.29 │ slli.d $a2, $a2, 0x2 > │ ldx.w $a2, $s8, $a2 > 12.77 │ st.w $a2, $sp, 724(0x2d4) > │ st.w $s0, $sp, 720(0x2d0) > 7.03 │ addi.d $a2, $sp, 720(0x2d0) > │ addi.d $a1, $a1, -1(0xfff) > 12.03 │ move $a2, $a3 > │ → bne $a1, $s3, -52(0x3ffcc) # 82ce8 > 2.50 │ addi.d $a0, $a0, 1(0x1) > > This patch fixes instruction association issues, such as associating > branch instructions with jump_ops instead of call_ops, and corrects > false instruction matches. It also implements branch instruction parsing > specifically for LoongArch. With this patch, we will be able to see the > arrowed line. > > 0.79 │3ec: ori $a1, $zero, 0x63 > │ move $a2, $zero > 10.32 │3f4:┌─→addi.d $a3, $a2, 1(0x1) > │ │ sltu $a4, $a3, $s7 > 10.44 │ │ masknez $a4, $s7, $a4 > │ │ sub.d $a3, $a3, $a4 > 14.17 │ │ st.d $a1, $fp, 24(0x18) > │ │ st.d $a3, $fp, 16(0x10) > 13.15 │ │ slli.d $a2, $a2, 0x2 > │ │ ldx.w $a2, $s8, $a2 > 11.00 │ │ st.w $a2, $sp, 724(0x2d4) > │ │ st.w $s0, $sp, 720(0x2d0) > 8.00 │ │ addi.d $a2, $sp, 720(0x2d0) > │ │ addi.d $a1, $a1, -1(0xfff) > 11.99 │ │ move $a2, $a3 > │ └──bne $a1, $s3, 3f4 > 3.17 │ addi.d $a0, $a0, 1(0x1) > > Signed-off-by: WANG Rui Just a nitpick. Otherwise looks good. > --- > .../arch/loongarch/annotate/instructions.c | 116 ++++++++++++++++-- > tools/perf/arch/s390/annotate/instructions.c | 3 - > tools/perf/util/annotate.c | 8 +- > 3 files changed, 109 insertions(+), 18 deletions(-) > > diff --git a/tools/perf/arch/loongarch/annotate/instructions.c b/tools/perf/arch/loongarch/annotate/instructions.c > index ab21bf122135..98e19c5366ac 100644 > --- a/tools/perf/arch/loongarch/annotate/instructions.c > +++ b/tools/perf/arch/loongarch/annotate/instructions.c > @@ -5,25 +5,115 @@ > * Copyright (C) 2020-2023 Loongson Technology Corporation Limited > */ > > +static int loongarch_call__parse(struct arch *arch, struct ins_operands *ops, struct map_symbol *ms) > +{ > + char *c, *endptr, *tok, *name; > + struct map *map = ms->map; > + struct addr_map_symbol target = { > + .ms = { .map = map, }, Looking here... > + }; > + > + c = strchr(ops->raw, '#'); > + if (c++ == NULL) > + return -1; > + > + ops->target.addr = strtoull(c, &endptr, 16); > + > + name = strchr(endptr, '<'); > + name++; > + > + if (arch->objdump.skip_functions_char && > + strchr(name, arch->objdump.skip_functions_char)) > + return -1; > + > + tok = strchr(name, '>'); > + if (tok == NULL) > + return -1; > + > + *tok = '\0'; > + ops->target.name = strdup(name); > + *tok = '>'; > + > + if (ops->target.name == NULL) > + return -1; > + > + target.addr = map__objdump_2mem(map, ops->target.addr); > + > + if (maps__find_ams(ms->maps, &target) == 0 && > + map__rip_2objdump(target.ms.map, map__map_ip(target.ms.map, target.addr)) == ops->target.addr) So the target.ms.map is 'map', right? Then we can reduce the line. > + ops->target.sym = target.ms.sym; > + > + return 0; > +} > + > +static struct ins_ops loongarch_call_ops = { > + .parse = loongarch_call__parse, > + .scnprintf = call__scnprintf, > +}; > + > +static int loongarch_jump__parse(struct arch *arch, struct ins_operands *ops, struct map_symbol *ms) > +{ > + struct map *map = ms->map; > + struct symbol *sym = ms->sym; > + struct addr_map_symbol target = { > + .ms = { .map = map, }, > + }; > + const char *c = strchr(ops->raw, '#'); > + u64 start, end; > + > + ops->raw_comment = strchr(ops->raw, arch->objdump.comment_char); > + ops->raw_func_start = strchr(ops->raw, '<'); > + > + if (ops->raw_func_start && c > ops->raw_func_start) > + c = NULL; > + > + if (c++ != NULL) > + ops->target.addr = strtoull(c, NULL, 16); > + else > + ops->target.addr = strtoull(ops->raw, NULL, 16); > + > + target.addr = map__objdump_2mem(map, ops->target.addr); > + start = map__unmap_ip(map, sym->start); > + end = map__unmap_ip(map, sym->end); > + > + ops->target.outside = target.addr < start || target.addr > end; > + > + if (maps__find_ams(ms->maps, &target) == 0 && > + map__rip_2objdump(target.ms.map, map__map_ip(target.ms.map, target.addr)) == ops->target.addr) Ditto. Thanks, Namhyung > + ops->target.sym = target.ms.sym; > + > + if (!ops->target.outside) { > + ops->target.offset = target.addr - start; > + ops->target.offset_avail = true; > + } else { > + ops->target.offset_avail = false; > + } > + > + return 0; > +} > + > +static struct ins_ops loongarch_jump_ops = { > + .parse = loongarch_jump__parse, > + .scnprintf = jump__scnprintf, > +}; > + > static > struct ins_ops *loongarch__associate_ins_ops(struct arch *arch, const char *name) > { > struct ins_ops *ops = NULL; > > - if (!strncmp(name, "beqz", 4) || > - !strncmp(name, "bnez", 4) || > - !strncmp(name, "beq", 3) || > - !strncmp(name, "bne", 3) || > - !strncmp(name, "blt", 3) || > - !strncmp(name, "bge", 3) || > - !strncmp(name, "bltu", 4) || > - !strncmp(name, "bgeu", 4) || > - !strncmp(name, "bl", 2)) > - ops = &call_ops; > - else if (!strncmp(name, "jirl", 4)) > + if (!strcmp(name, "bl")) > + ops = &loongarch_call_ops; > + else if (!strcmp(name, "jirl")) > ops = &ret_ops; > - else if (name[0] == 'b') > - ops = &jump_ops; > + else if (!strcmp(name, "b") || > + !strncmp(name, "beq", 3) || > + !strncmp(name, "bne", 3) || > + !strncmp(name, "blt", 3) || > + !strncmp(name, "bge", 3) || > + !strncmp(name, "bltu", 4) || > + !strncmp(name, "bgeu", 4)) > + ops = &loongarch_jump_ops; > else > return NULL; > > diff --git a/tools/perf/arch/s390/annotate/instructions.c b/tools/perf/arch/s390/annotate/instructions.c > index de925b0e35ce..da5aa3e1f04c 100644 > --- a/tools/perf/arch/s390/annotate/instructions.c > +++ b/tools/perf/arch/s390/annotate/instructions.c > @@ -45,9 +45,6 @@ static int s390_call__parse(struct arch *arch, struct ins_operands *ops, > return 0; > } > > -static int call__scnprintf(struct ins *ins, char *bf, size_t size, > - struct ins_operands *ops, int max_ins_name); > - > static struct ins_ops s390_call_ops = { > .parse = s390_call__parse, > .scnprintf = call__scnprintf, > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index cdd1924a4418..ad40adbd8895 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -61,6 +61,10 @@ static regex_t file_lineno; > static struct ins_ops *ins__find(struct arch *arch, const char *name); > static void ins__sort(struct arch *arch); > static int disasm_line__parse(char *line, const char **namep, char **rawp); > +static int call__scnprintf(struct ins *ins, char *bf, size_t size, > + struct ins_operands *ops, int max_ins_name); > +static int jump__scnprintf(struct ins *ins, char *bf, size_t size, > + struct ins_operands *ops, int max_ins_name); > > struct arch { > const char *name; > @@ -323,7 +327,7 @@ static struct ins_ops call_ops = { > > bool ins__is_call(const struct ins *ins) > { > - return ins->ops == &call_ops || ins->ops == &s390_call_ops; > + return ins->ops == &call_ops || ins->ops == &s390_call_ops || ins->ops == &loongarch_call_ops; > } > > /* > @@ -464,7 +468,7 @@ static struct ins_ops jump_ops = { > > bool ins__is_jump(const struct ins *ins) > { > - return ins->ops == &jump_ops; > + return ins->ops == &jump_ops || ins->ops == &loongarch_jump_ops; > } > > static int comment__symbol(char *raw, char *comment, u64 *addrp, char **namep) > -- > 2.41.0 >