* [PATCH] perf annotate: Fix instruction association and parsing for LoongArch
@ 2023-06-20 13:20 WANG Rui
2023-06-20 18:42 ` Namhyung Kim
2023-06-21 17:28 ` Namhyung Kim
0 siblings, 2 replies; 5+ messages in thread
From: WANG Rui @ 2023-06-20 13:20 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Huacai Chen
Cc: loongarch, Tiezhu Yang, WANG Xuerui, linux-kernel,
linux-perf-users, loongson-kernel, WANG Rui
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 <test::bench::Bencher::iter+0x3f4>
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 <wangrui@loongson.cn>
---
.../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, },
+ };
+
+ 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)
+ 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)
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf annotate: Fix instruction association and parsing for LoongArch
2023-06-20 13:20 [PATCH] perf annotate: Fix instruction association and parsing for LoongArch WANG Rui
@ 2023-06-20 18:42 ` Namhyung Kim
2023-06-21 4:10 ` WANG Rui
2023-06-21 17:28 ` Namhyung Kim
1 sibling, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2023-06-20 18:42 UTC (permalink / raw)
To: WANG Rui
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Huacai Chen,
loongarch, Tiezhu Yang, WANG Xuerui, linux-kernel,
linux-perf-users, loongson-kernel
Hello,
On Tue, Jun 20, 2023 at 6:21 AM WANG Rui <wangrui@loongson.cn> 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 <test::bench::Bencher::iter+0x3f4>
> 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 <wangrui@loongson.cn>
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf annotate: Fix instruction association and parsing for LoongArch
2023-06-20 18:42 ` Namhyung Kim
@ 2023-06-21 4:10 ` WANG Rui
2023-06-21 4:54 ` Namhyung Kim
0 siblings, 1 reply; 5+ messages in thread
From: WANG Rui @ 2023-06-21 4:10 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Huacai Chen,
loongarch, Tiezhu Yang, WANG Xuerui, linux-kernel,
linux-perf-users, loongson-kernel
Hello Namhyung,
On Wed, Jun 21, 2023 at 2:42 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > +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.
toos/perf/util/maps.c:
> int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams)
> {
> if (ams->addr < map__start(ams->ms.map) || ams->addr >= map__end(ams->ms.map)) {
> if (maps == NULL)
> return -1;
> ams->ms.map = maps__find(maps, ams->addr);
Is it possible that `target.ms.map` might be reassigned within the
`maps_find_ams` function in this case?
> if (ams->ms.map == NULL)
> return -1;
> }
>
> ams->al_addr = map__map_ip(ams->ms.map, ams->addr);
> ams->ms.sym = map__find_symbol(ams->ms.map, ams->al_addr);
>
> return ams->ms.sym ? 0 : -1;
> }
Thanks!
--
WANG Rui
Loongson Technology Corporation Limited
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf annotate: Fix instruction association and parsing for LoongArch
2023-06-21 4:10 ` WANG Rui
@ 2023-06-21 4:54 ` Namhyung Kim
0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2023-06-21 4:54 UTC (permalink / raw)
To: WANG Rui
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Huacai Chen,
loongarch, Tiezhu Yang, WANG Xuerui, linux-kernel,
linux-perf-users, loongson-kernel
On Tue, Jun 20, 2023 at 9:37 PM WANG Rui <wangrui@loongson.cn> wrote:
>
> Hello Namhyung,
>
> On Wed, Jun 21, 2023 at 2:42 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > +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.
>
> toos/perf/util/maps.c:
>
> > int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams)
> > {
> > if (ams->addr < map__start(ams->ms.map) || ams->addr >= map__end(ams->ms.map)) {
> > if (maps == NULL)
> > return -1;
> > ams->ms.map = maps__find(maps, ams->addr);
>
> Is it possible that `target.ms.map` might be reassigned within the
> `maps_find_ams` function in this case?
Ah, ok. Missed that part. It can change if the target is from a
different library.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
>
> > if (ams->ms.map == NULL)
> > return -1;
> > }
> >
> > ams->al_addr = map__map_ip(ams->ms.map, ams->addr);
> > ams->ms.sym = map__find_symbol(ams->ms.map, ams->al_addr);
> >
> > return ams->ms.sym ? 0 : -1;
> > }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf annotate: Fix instruction association and parsing for LoongArch
2023-06-20 13:20 [PATCH] perf annotate: Fix instruction association and parsing for LoongArch WANG Rui
2023-06-20 18:42 ` Namhyung Kim
@ 2023-06-21 17:28 ` Namhyung Kim
1 sibling, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2023-06-21 17:28 UTC (permalink / raw)
To: WANG Rui
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Huacai Chen,
loongarch, Tiezhu Yang, WANG Xuerui, linux-kernel,
linux-perf-users, loongson-kernel
On Tue, Jun 20, 2023 at 6:21 AM WANG Rui <wangrui@loongson.cn> 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 <test::bench::Bencher::iter+0x3f4>
> 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 <wangrui@loongson.cn>
Applied to perf-tools-next, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-21 17:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-20 13:20 [PATCH] perf annotate: Fix instruction association and parsing for LoongArch WANG Rui
2023-06-20 18:42 ` Namhyung Kim
2023-06-21 4:10 ` WANG Rui
2023-06-21 4:54 ` Namhyung Kim
2023-06-21 17:28 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).