Since the "ins.name" is not set while using raw instruction, perf annotate with insn-stat gives wrong data:
Result from "./perf annotate --data-type --insn-stat": Annotate Instruction stats total 615, ok 419 (68.1%), bad 196 (31.9%) Name : Good Bad ----------------------------------------------------------- : 419 196 Patch sets "dl->ins.name" in arch specific function "check_ppc_insn" while initialising "struct disasm_line". Also update "ins_find" function to pass "struct disasm_line" as a parameter so as to set its name field in arch specific call. With the patch changes: Annotate Instruction stats total 609, ok 446 (73.2%), bad 163 (26.8%) Name/opcode : Good Bad ----------------------------------------------------------- 58 : 323 80 32 : 49 43 34 : 33 11 OP_31_XOP_LDX : 8 20 40 : 23 0 OP_31_XOP_LWARX : 5 1 OP_31_XOP_LWZX : 2 3 OP_31_XOP_LDARX : 3 0 33 : 0 2 OP_31_XOP_LBZX : 0 1 OP_31_XOP_LWAX : 0 1 OP_31_XOP_LHZX : 0 1 Reviewed-and-tested-by: Kajol Jain<kj...@linux.ibm.com> Reviewed-by: Namhyung Kim <namhy...@kernel.org> Signed-off-by: Athira Rajeev <atraj...@linux.vnet.ibm.com> --- .../perf/arch/powerpc/annotate/instructions.c | 18 +++++++++++++++--- tools/perf/builtin-annotate.c | 4 ++-- tools/perf/util/annotate.c | 2 +- tools/perf/util/disasm.c | 10 +++++----- tools/perf/util/disasm.h | 2 +- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/tools/perf/arch/powerpc/annotate/instructions.c b/tools/perf/arch/powerpc/annotate/instructions.c index af1032572bf3..ede9eeade0ab 100644 --- a/tools/perf/arch/powerpc/annotate/instructions.c +++ b/tools/perf/arch/powerpc/annotate/instructions.c @@ -189,8 +189,9 @@ static int cmp_offset(const void *a, const void *b) return (val1->value - val2->value); } -static struct ins_ops *check_ppc_insn(u32 raw_insn) +static struct ins_ops *check_ppc_insn(struct disasm_line *dl) { + int raw_insn = dl->raw.raw_insn; int opcode = PPC_OP(raw_insn); int mem_insn_31 = PPC_21_30(raw_insn); struct insn_offset *ret; @@ -198,19 +199,30 @@ static struct ins_ops *check_ppc_insn(u32 raw_insn) "OP_31_INSN", mem_insn_31 }; + char name_insn[32]; /* * Instructions with opcode 32 to 63 are memory * instructions in powerpc */ if ((opcode & 0x20)) { + /* + * Set name in case of raw instruction to + * opcode to be used in insn-stat + */ + if (!strlen(dl->ins.name)) { + sprintf(name_insn, "%d", opcode); + dl->ins.name = strdup(name_insn); + } return &load_store_ops; } else if (opcode == 31) { /* Check for memory instructions with opcode 31 */ ret = bsearch(&mem_insns_31_opcode, ins_array, ARRAY_SIZE(ins_array), sizeof(ins_array[0]), cmp_offset); - if (ret != NULL) + if (ret) { + if (!strlen(dl->ins.name)) + dl->ins.name = strdup(ret->name); return &load_store_ops; - else { + } else { mem_insns_31_opcode.value = PPC_22_30(raw_insn); ret = bsearch(&mem_insns_31_opcode, arithmetic_ins_op_31, ARRAY_SIZE(arithmetic_ins_op_31), sizeof(arithmetic_ins_op_31[0]), cmp_offset); diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index b10b7f005658..cf60392b1c19 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -396,10 +396,10 @@ static void print_annotate_item_stat(struct list_head *head, const char *title) printf("total %d, ok %d (%.1f%%), bad %d (%.1f%%)\n\n", total, total_good, 100.0 * total_good / (total ?: 1), total_bad, 100.0 * total_bad / (total ?: 1)); - printf(" %-10s: %5s %5s\n", "Name", "Good", "Bad"); + printf(" %-20s: %5s %5s\n", "Name/opcode", "Good", "Bad"); printf("-----------------------------------------------------------\n"); list_for_each_entry(istat, head, list) - printf(" %-10s: %5d %5d\n", istat->name, istat->good, istat->bad); + printf(" %-20s: %5d %5d\n", istat->name, istat->good, istat->bad); printf("\n"); } diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ce99db291c5e..a2ee4074f768 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -2229,7 +2229,7 @@ static struct annotated_item_stat *annotate_data_stat(struct list_head *head, return NULL; istat->name = strdup(name); - if (istat->name == NULL) { + if ((istat->name == NULL) || (!strlen(istat->name))) { free(istat); return NULL; } diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c index 63681df6482b..cd283c42195c 100644 --- a/tools/perf/util/disasm.c +++ b/tools/perf/util/disasm.c @@ -859,7 +859,7 @@ static void ins__sort(struct arch *arch) qsort(arch->instructions, nmemb, sizeof(struct ins), ins__cmp); } -static struct ins_ops *__ins__find(struct arch *arch, const char *name, u32 raw_insn) +static struct ins_ops *__ins__find(struct arch *arch, const char *name, struct disasm_line *dl) { struct ins *ins; const int nmemb = arch->nr_instructions; @@ -871,7 +871,7 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name, u32 raw_ */ struct ins_ops *ops; - ops = check_ppc_insn(raw_insn); + ops = check_ppc_insn(dl); if (ops) return ops; } @@ -905,9 +905,9 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name, u32 raw_ return ins ? ins->ops : NULL; } -struct ins_ops *ins__find(struct arch *arch, const char *name, u32 raw_insn) +struct ins_ops *ins__find(struct arch *arch, const char *name, struct disasm_line *dl) { - struct ins_ops *ops = __ins__find(arch, name, raw_insn); + struct ins_ops *ops = __ins__find(arch, name, dl); if (!ops && arch->associate_instruction_ops) ops = arch->associate_instruction_ops(arch, name); @@ -917,7 +917,7 @@ struct ins_ops *ins__find(struct arch *arch, const char *name, u32 raw_insn) static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map_symbol *ms) { - dl->ins.ops = ins__find(arch, dl->ins.name, dl->raw.raw_insn); + dl->ins.ops = ins__find(arch, dl->ins.name, dl); if (!dl->ins.ops) return; diff --git a/tools/perf/util/disasm.h b/tools/perf/util/disasm.h index c1bb1e484bfb..f56beedeb9da 100644 --- a/tools/perf/util/disasm.h +++ b/tools/perf/util/disasm.h @@ -105,7 +105,7 @@ struct annotate_args { struct arch *arch__find(const char *name); bool arch__is(struct arch *arch, const char *name); -struct ins_ops *ins__find(struct arch *arch, const char *name, u32 raw_insn); +struct ins_ops *ins__find(struct arch *arch, const char *name, struct disasm_line *dl); int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops, int max_ins_name); -- 2.43.0