Re: [PATCH V2 6/9] tools/perf: Update instruction tracking for powerpc

2024-05-23 Thread Athira Rajeev



> On 7 May 2024, at 3:22 PM, Christophe Leroy  
> wrote:
> 
> 
> 
> Le 06/05/2024 à 14:19, Athira Rajeev a écrit :
>> Add instruction tracking function "update_insn_state_powerpc" for
>> powerpc. Example sequence in powerpc:
>> 
>> ld  r10,264(r3)
>> mr  r31,r3
>> <
>> ld  r9,312(r31)
> 
> Your approach looks fragile.
> 
> mr is a simplified instruction which in fact is  "or r31, r3, r3"
> 
> By the way, the compiler sometimes does it different, like below:
> 
> lwz r10,264(r3)
> addi r31, r3, 312
> lwz r9, 0(r31)
> 
> And what about sequences with lwzu ?
Hi Christophe,

This patch added “mr”. In next patch in same series, add instruction also is 
added to instruction tracking.
I will be posting a V3 with some changes to the logic for annotating add 
instructions.

Thanks
Athira
> 
>> 
>> Consider ithe sample is pointing to: "ld r9,312(r31)".
>> Here the memory reference is hit at "312(r31)" where 312 is the offset
>> and r31 is the source register. Previous instruction sequence shows that
>> register state of r3 is moved to r31. So to identify the data type for r31
>> access, the previous instruction ("mr") needs to be tracked and the
>> state type entry has to be updated. Current instruction tracking support
>> in perf tools infrastructure is specific to x86. Patch adds this for
>> powerpc and adds "mr" instruction to be tracked.
>> 
>> Signed-off-by: Athira Rajeev 



Re: [PATCH V2 6/9] tools/perf: Update instruction tracking for powerpc

2024-05-07 Thread Christophe Leroy


Le 06/05/2024 à 14:19, Athira Rajeev a écrit :
> Add instruction tracking function "update_insn_state_powerpc" for
> powerpc. Example sequence in powerpc:
> 
> ld  r10,264(r3)
> mr  r31,r3
> <
> ld  r9,312(r31)

Your approach looks fragile.

mr is a simplified instruction which in fact is  "or r31, r3, r3"

By the way, the compiler sometimes does it different, like below:

lwz r10,264(r3)
addir31, r3, 312
lwz r9, 0(r31)

And what about sequences with lwzu ?

> 
> Consider ithe sample is pointing to: "ld r9,312(r31)".
> Here the memory reference is hit at "312(r31)" where 312 is the offset
> and r31 is the source register. Previous instruction sequence shows that
> register state of r3 is moved to r31. So to identify the data type for r31
> access, the previous instruction ("mr") needs to be tracked and the
> state type entry has to be updated. Current instruction tracking support
> in perf tools infrastructure is specific to x86. Patch adds this for
> powerpc and adds "mr" instruction to be tracked.
> 
> Signed-off-by: Athira Rajeev 


[PATCH V2 6/9] tools/perf: Update instruction tracking for powerpc

2024-05-06 Thread Athira Rajeev
Add instruction tracking function "update_insn_state_powerpc" for
powerpc. Example sequence in powerpc:

ld  r10,264(r3)
mr  r31,r3
<
ld  r9,312(r31)

Consider ithe sample is pointing to: "ld r9,312(r31)".
Here the memory reference is hit at "312(r31)" where 312 is the offset
and r31 is the source register. Previous instruction sequence shows that
register state of r3 is moved to r31. So to identify the data type for r31
access, the previous instruction ("mr") needs to be tracked and the
state type entry has to be updated. Current instruction tracking support
in perf tools infrastructure is specific to x86. Patch adds this for
powerpc and adds "mr" instruction to be tracked.

Signed-off-by: Athira Rajeev 
---
 .../perf/arch/powerpc/annotate/instructions.c | 63 +++
 tools/perf/util/annotate-data.c   |  9 ++-
 tools/perf/util/disasm.c  |  1 +
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/powerpc/annotate/instructions.c 
b/tools/perf/arch/powerpc/annotate/instructions.c
index a3f423c27cae..cce7023951fe 100644
--- a/tools/perf/arch/powerpc/annotate/instructions.c
+++ b/tools/perf/arch/powerpc/annotate/instructions.c
@@ -49,6 +49,69 @@ static struct ins_ops 
*powerpc__associate_instruction_ops(struct arch *arch, con
return ops;
 }
 
+/*
+ * Instruction tracking function to track register state moves.
+ * Example sequence:
+ *ld  r10,264(r3)
+ *mr  r31,r3
+ *<
+ *ld  r9,312(r31)
+ *
+ * Previous instruction sequence shows that register state of r3
+ * is moved to r31. update_insn_state_powerpc tracks these state
+ * changes
+ */
+#ifdef HAVE_DWARF_SUPPORT
+static void update_insn_state_powerpc(struct type_state *state,
+   struct data_loc_info *dloc, Dwarf_Die *cu_die __maybe_unused,
+   struct disasm_line *dl)
+{
+   struct annotated_insn_loc loc;
+   struct annotated_op_loc *src = &loc.ops[INSN_OP_SOURCE];
+   struct annotated_op_loc *dst = &loc.ops[INSN_OP_TARGET];
+   struct type_state_reg *tsr;
+   u32 insn_offset = dl->al.offset;
+
+   if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
+   return;
+
+   if (strncmp(dl->ins.name, "mr", 2))
+   return;
+
+   if (!strncmp(dl->ins.name, "mr", 2)) {
+   int src_reg = src->reg1;
+
+   src->reg1 = dst->reg1;
+   dst->reg1 = src_reg;
+   }
+
+   if (!has_reg_type(state, dst->reg1))
+   return;
+
+   tsr = &state->regs[dst->reg1];
+
+   if (!has_reg_type(state, src->reg1) ||
+   !state->regs[src->reg1].ok) {
+   tsr->ok = false;
+   return;
+   }
+
+   tsr->type = state->regs[src->reg1].type;
+   tsr->kind = state->regs[src->reg1].kind;
+   tsr->ok = true;
+
+   pr_debug("mov [%x] reg%d -> reg%d",
+   insn_offset, src->reg1, dst->reg1);
+   pr_debug_type_name(&tsr->type, tsr->kind);
+}
+#else /* HAVE_DWARF_SUPPORT */
+static void update_insn_state_powerpc(struct type_state *state __maybe_unused, 
struct data_loc_info *dloc __maybe_unused,
+   Dwarf_Die *cu_die __maybe_unused, struct disasm_line *dl 
__maybe_unused)
+{
+   return;
+}
+#endif /* HAVE_DWARF_SUPPORT */
+
 static int powerpc__annotate_init(struct arch *arch, char *cpuid 
__maybe_unused)
 {
if (!arch->initialized) {
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 9d6d4f472c85..e22ba35c93b2 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1079,6 +1079,13 @@ static int find_data_type_insn(struct data_loc_info 
*dloc,
return ret;
 }
 
+static int arch_supports_insn_tracking(struct data_loc_info *dloc)
+{
+   if ((arch__is(dloc->arch, "x86")) || (arch__is(dloc->arch, "powerpc")))
+   return 1;
+   return 0;
+}
+
 /*
  * Construct a list of basic blocks for each scope with variables and try to 
find
  * the data type by updating a type state table through instructions.
@@ -1093,7 +1100,7 @@ static int find_data_type_block(struct data_loc_info 
*dloc,
int ret = -1;
 
/* TODO: other architecture support */
-   if (!arch__is(dloc->arch, "x86"))
+   if (!arch_supports_insn_tracking(dloc))
return -1;
 
prev_dst_ip = dst_ip = dloc->ip;
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index f41a0fadeab4..ac6b8b8da38a 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -151,6 +151,7 @@ static struct arch architectures[] = {
{
.name = "powerpc",
.init = powerpc__annotate_init,
+   .update_insn_state = update_insn_state_powerpc,
},
{
.name = "riscv64",
-- 
2.43.0