From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
To: acme@kernel.org, jolsa@kernel.org, adrian.hunter@intel.com,
irogers@google.com, namhyung@kernel.org,
segher@kernel.crashing.org, christophe.leroy@csgroup.eu
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, akanksha@linux.ibm.com,
maddy@linux.ibm.com, atrajeev@linux.vnet.ibm.com,
kjain@linux.ibm.com, hbathini@linux.ibm.com,
disgoel@linux.vnet.ibm.com
Subject: [PATCH V8 15/15] tools/perf: Set instruction name to be used with insn-stat when using raw instruction
Date: Thu, 18 Jul 2024 14:13:58 +0530 [thread overview]
Message-ID: <20240718084358.72242-16-atrajeev@linux.vnet.ibm.com> (raw)
In-Reply-To: <20240718084358.72242-1-atrajeev@linux.vnet.ibm.com>
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<kjain@linux.ibm.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Athira Rajeev <atrajeev@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
WARNING: multiple messages have this Message-ID (diff)
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
To: acme@kernel.org, jolsa@kernel.org, adrian.hunter@intel.com,
irogers@google.com, namhyung@kernel.org,
segher@kernel.crashing.org, christophe.leroy@csgroup.eu
Cc: atrajeev@linux.vnet.ibm.com, kjain@linux.ibm.com,
linux-kernel@vger.kernel.org, akanksha@linux.ibm.com,
linux-perf-users@vger.kernel.org, maddy@linux.ibm.com,
disgoel@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
hbathini@linux.ibm.com
Subject: [PATCH V8 15/15] tools/perf: Set instruction name to be used with insn-stat when using raw instruction
Date: Thu, 18 Jul 2024 14:13:58 +0530 [thread overview]
Message-ID: <20240718084358.72242-16-atrajeev@linux.vnet.ibm.com> (raw)
In-Reply-To: <20240718084358.72242-1-atrajeev@linux.vnet.ibm.com>
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<kjain@linux.ibm.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Athira Rajeev <atrajeev@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
next prev parent reply other threads:[~2024-07-18 8:46 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-18 8:43 [PATCH V8 00/15] Add data type profiling support for powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 01/15] tools/perf: Move the data structures related to register type to header file Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 02/15] tools/perf: Add "update_insn_state" callback function to handle arch specific instruction tracking Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 03/15] tools/perf: Update TYPE_STATE_MAX_REGS to include max of regs in powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-23 19:06 ` Arnaldo Carvalho de Melo
2024-07-23 19:06 ` Arnaldo Carvalho de Melo
2024-07-24 5:38 ` Athira Rajeev
2024-07-24 5:38 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 04/15] tools/perf: Add disasm_line__parse to parse raw instruction for powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-23 19:14 ` Arnaldo Carvalho de Melo
2024-07-23 19:14 ` Arnaldo Carvalho de Melo
2024-07-24 5:38 ` Athira Rajeev
2024-07-24 5:38 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 05/15] tools/perf: Add support to capture and parse raw instruction in powerpc using dso__data_read_offset utility Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 06/15] tools/perf: Update parameters for reg extract functions to use raw instruction on powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 07/15] tools/perf: Add parse function for memory instructions in powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 08/15] tools/perf: Add support to identify memory instructions of opcode 31 " Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 09/15] tools/perf: Add some of the arithmetic instructions to support instruction tracking " Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 10/15] tools/perf: Add more instructions for instruction tracking Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 11/15] tools/perf: Update instruction tracking for powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 12/15] tools/perf: Make capstone_init non-static so that it can be used during symbol disassemble Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 13/15] tools/perf: Use capstone_init and remove open_capstone_handle from disasm.c Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-18 8:43 ` [PATCH V8 14/15] tools/perf: Add support to use libcapstone in powerpc Athira Rajeev
2024-07-18 8:43 ` Athira Rajeev
2024-07-24 21:00 ` Arnaldo Carvalho de Melo
2024-07-24 21:00 ` Arnaldo Carvalho de Melo
2024-07-26 12:35 ` Athira Rajeev
2024-07-26 12:35 ` Athira Rajeev
2024-07-31 13:51 ` kernel test robot
2024-07-31 13:51 ` kernel test robot
2024-07-18 8:43 ` Athira Rajeev [this message]
2024-07-18 8:43 ` [PATCH V8 15/15] tools/perf: Set instruction name to be used with insn-stat when using raw instruction Athira Rajeev
2024-07-23 20:07 ` [PATCH V8 00/15] Add data type profiling support for powerpc Arnaldo Carvalho de Melo
2024-07-23 20:07 ` Arnaldo Carvalho de Melo
2024-07-24 7:59 ` Athira Rajeev
2024-07-24 7:59 ` Athira Rajeev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240718084358.72242-16-atrajeev@linux.vnet.ibm.com \
--to=atrajeev@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=akanksha@linux.ibm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=disgoel@linux.vnet.ibm.com \
--cc=hbathini@linux.ibm.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kjain@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=namhyung@kernel.org \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.