[PATCH 0/3] Add data type profiling support for powerpc

[PATCH 0/3] Add data type profiling support for powerpc

[Athira Rajeev]

From: Akanksha J N <Akanksha.J.N@ibm.com>

The patchset from Namhyung added support for data type profiling
in perf tool. This enabled support to associate PMU samples to data
types they refer using DWARF debug information. With the upstream
perf, currently it possible to run perf report or perf annotate to
view the data type information on x86.

This patchset includes changes need to enable data type profiling
support for powerpc. Main change are:
1. powerpc instruction nmemonic table to associate load/store
instructions with move_ops which is use to identify if instruction
is a memory access one.
2. To get register number and access offset from the given
instruction, code uses fields from "struct arch" -> objump.
Add entry for powerpc here.
3. A get_arch_regnum to return register number from the
register name string.

These three patches are the basic foundational patches.
With these changes, data types is getting identified for kernel
and user-space samples. There are still samples, which comes as
"unknown" and needs to be checked. We plan to get those addressed
in follow up. With the current patchset:

# ./perf record -a -e mem-loads sleep 1
# ./perf report -s type,typeoff --hierarchy --group --stdio
Snippet of logs:
#
# Total Lost Samples: 0
#
# Samples: 277  of events 'mem-loads, dummy:u'
# Event count (approx.): 149813
#
#            Overhead  Data Type / Data Type Offset
# ...................  ............................
#
    65.93%   0.00%     (unknown)
       65.93%   0.00%     (unknown) +0 (no field)
     8.19%   0.00%     struct vm_area_struct
        8.19%   0.00%     struct vm_area_struct +136 (vm_file)
     4.53%   0.00%     struct rq
        3.14%   0.00%     struct rq +0 (__lock.raw_lock.val)
        0.83%   0.00%     struct rq +3216 (avg_irq.runnable_sum)
        0.24%   0.00%     struct rq +4 (nr_running)
        0.14%   0.00%     struct rq +12 (nr_preferred_running)
        0.12%   0.00%     struct rq +2760 (sd)
        0.06%   0.00%     struct rq +3368 (prev_steal_time_rq)
        0.01%   0.00%     struct rq +2592 (curr)
     3.53%   0.00%     struct rb_node
        3.53%   0.00%     struct rb_node +0 (__rb_parent_color)
     3.43%   0.00%     struct slab
        3.43%   0.00%     struct slab +32 (freelist)
     3.30%   0.00%     unsigned int
        3.30%   0.00%     unsigned int +0 (no field)
     3.22%   0.00%     struct vm_fault
        3.22%   0.00%     struct vm_fault +48 (pmd)
     2.55%   0.00%     unsigned char
        2.55%   0.00%     unsigned char +0 (no field)
     1.06%   0.00%     struct task_struct
        1.06%   0.00%     struct task_struct +4 (thread_info.cpu)
     0.92%   0.00%     void*    
        0.92%   0.00%     void* +0 (no field)
     0.74%   0.00%     __int128 unsigned
        0.74%   0.00%     __int128 unsigned +8 (no field)
     0.59%   0.00%     struct perf_event
        0.54%   0.00%     struct perf_event +552 (ctx)
        0.04%   0.00%     struct perf_event +152 (pmu)
     0.20%   0.00%     struct sched_entity
        0.20%   0.00%     struct sched_entity +0 (load.weight)
     0.18%   0.00%     struct cfs_rq
        0.18%   0.00%     struct cfs_rq +96 (curr)

Thanks
Athira

Athira Rajeev (3):
  tools/perf/arch/powerpc: Add load/store in powerpc annotate
    instructions for data type profling
  tools/erf/util/annotate: Set register_char and memory_ref_char for
    powerpc
  tools/perf/arch/powerc: Add get_arch_regnum for powerpc

 .../perf/arch/powerpc/annotate/instructions.c | 66 +++++++++++++++++++
 tools/perf/arch/powerpc/util/dwarf-regs.c     | 29 ++++++++
 tools/perf/util/annotate.c                    |  5 ++
 3 files changed, 100 insertions(+)

-- 
2.43.0

[PATCH 1/3] tools/perf/arch/powerpc: Add load/store in powerpc annotate instructions for data type profling

[Athira Rajeev]

Add powerpc instruction nmemonic table to associate load/store
instructions with move_ops. mov_ops is used to identify mem_type
to associate instruction with data type and offset. Also initialize
and allocate arch specific fields for nr_instructions, instructions and
nr_instructions_allocate.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 .../perf/arch/powerpc/annotate/instructions.c | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tools/perf/arch/powerpc/annotate/instructions.c b/tools/perf/arch/powerpc/annotate/instructions.c
index a3f423c27cae..07af4442be38 100644
--- a/tools/perf/arch/powerpc/annotate/instructions.c
+++ b/tools/perf/arch/powerpc/annotate/instructions.c
@@ -1,6 +1,65 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/compiler.h>
 
+/*
+ * powerpc instruction nmemonic table to associate load/store instructions with
+ * move_ops. mov_ops is used to identify mem_type to associate instruction with
+ * data type and offset.
+ */
+static struct ins powerpc__instructions[] = {
+	{ .name = "lbz",	.ops = &mov_ops,  },
+	{ .name = "lbzx",	.ops = &mov_ops,  },
+	{ .name = "lbzu",	.ops = &mov_ops,  },
+	{ .name = "lbzux",	.ops = &mov_ops,  },
+	{ .name = "lhz",	.ops = &mov_ops,  },
+	{ .name = "lhzx",	.ops = &mov_ops,  },
+	{ .name = "lhzu",	.ops = &mov_ops,  },
+	{ .name = "lhzux",	.ops = &mov_ops,  },
+	{ .name = "lha",	.ops = &mov_ops,  },
+	{ .name = "lhax",	.ops = &mov_ops,  },
+	{ .name = "lhau",	.ops = &mov_ops,  },
+	{ .name = "lhaux",	.ops = &mov_ops,  },
+	{ .name = "lwz",	.ops = &mov_ops,  },
+	{ .name = "lwzx",	.ops = &mov_ops,  },
+	{ .name = "lwzu",	.ops = &mov_ops,  },
+	{ .name = "lwzux",	.ops = &mov_ops,  },
+	{ .name = "lwa",	.ops = &mov_ops,  },
+	{ .name = "lwax",	.ops = &mov_ops,  },
+	{ .name = "lwaux",	.ops = &mov_ops,  },
+	{ .name = "ld",		.ops = &mov_ops,  },
+	{ .name = "ldx",	.ops = &mov_ops,  },
+	{ .name = "ldu",	.ops = &mov_ops,  },
+	{ .name = "ldux",	.ops = &mov_ops,  },
+	{ .name = "stb",	.ops = &mov_ops,  },
+	{ .name = "stbx",	.ops = &mov_ops,  },
+	{ .name = "stbu",	.ops = &mov_ops,  },
+	{ .name = "stbux",	.ops = &mov_ops,  },
+	{ .name = "sth",	.ops = &mov_ops,  },
+	{ .name = "sthx",	.ops = &mov_ops,  },
+	{ .name = "sthu",	.ops = &mov_ops,  },
+	{ .name = "sthux",	.ops = &mov_ops,  },
+	{ .name = "stw",	.ops = &mov_ops,  },
+	{ .name = "stwx",	.ops = &mov_ops,  },
+	{ .name = "stwu",	.ops = &mov_ops,  },
+	{ .name = "stwux",	.ops = &mov_ops,  },
+	{ .name = "std",	.ops = &mov_ops,  },
+	{ .name = "stdx",	.ops = &mov_ops,  },
+	{ .name = "stdu",	.ops = &mov_ops,  },
+	{ .name = "stdux",	.ops = &mov_ops,  },
+	{ .name = "lhbrx",	.ops = &mov_ops,  },
+	{ .name = "sthbrx",	.ops = &mov_ops,  },
+	{ .name = "lwbrx",	.ops = &mov_ops,  },
+	{ .name = "stwbrx",	.ops = &mov_ops,  },
+	{ .name = "ldbrx",	.ops = &mov_ops,  },
+	{ .name = "stdbrx",	.ops = &mov_ops,  },
+	{ .name = "lmw",	.ops = &mov_ops,  },
+	{ .name = "stmw",	.ops = &mov_ops,  },
+	{ .name = "lswi",	.ops = &mov_ops,  },
+	{ .name = "lswx",	.ops = &mov_ops,  },
+	{ .name = "stswi",	.ops = &mov_ops,  },
+	{ .name = "stswx",	.ops = &mov_ops,  },
+};
+
 static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, const char *name)
 {
 	int i;
@@ -52,6 +111,13 @@ static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, con
 static int powerpc__annotate_init(struct arch *arch, char *cpuid __maybe_unused)
 {
 	if (!arch->initialized) {
+		arch->nr_instructions = ARRAY_SIZE(powerpc__instructions);
+		arch->instructions = calloc(arch->nr_instructions, sizeof(struct ins));
+		if (arch->instructions == NULL)
+			return -ENOMEM;
+
+		memcpy(arch->instructions, (struct ins *)powerpc__instructions, sizeof(struct ins) * arch->nr_instructions);
+		arch->nr_instructions_allocated = arch->nr_instructions;
 		arch->initialized = true;
 		arch->associate_instruction_ops = powerpc__associate_instruction_ops;
 		arch->objdump.comment_char      = '#';
-- 
2.43.0

[PATCH 2/3] tools/erf/util/annotate: Set register_char and memory_ref_char for powerpc

[Athira Rajeev]

To identify if the instruction has any memory reference,
"memory_ref_char" field needs to be set for specific architecture.
Example memory instruction:
lwz     r10,0(r9)

Here "(" is the memory_ref_char. Set this as part of arch->objdump

To get register number and access offset from the given instruction,
arch->objdump.register_char is used. In case of powerpc, the register
char is "r" (since reg names are r0 to r31). Hence set register_char
as "r".

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/util/annotate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ac002d907d81..d69bd6edafcb 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -216,6 +216,11 @@ static struct arch architectures[] = {
 	{
 		.name = "powerpc",
 		.init = powerpc__annotate_init,
+		.objdump =  {
+			.comment_char = '#',
+			.register_char = 'r',
+			.memory_ref_char = '(',
+		},
 	},
 	{
 		.name = "riscv64",
-- 
2.43.0

[PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Athira Rajeev]

The function get_dwarf_regnum() returns a DWARF register number
from a register name string. This calls arch specific function
get_arch_regnum to return register number for corresponding arch.
Add mappings for register name to register number in powerpc code:
arch/powerpc/util/dwarf-regs.c

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c b/tools/perf/arch/powerpc/util/dwarf-regs.c
index 0c4f4caf53ac..d955e3e577ea 100644
--- a/tools/perf/arch/powerpc/util/dwarf-regs.c
+++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
@@ -98,3 +98,32 @@ int regs_query_register_offset(const char *name)
 			return roff->ptregs_offset;
 	return -EINVAL;
 }
+
+struct dwarf_regs_idx {
+	const char *name;
+	int idx;
+};
+
+static const struct dwarf_regs_idx powerpc_regidx_table[] = {
+	{ "r0", 0 }, { "r1", 1 }, { "r2", 2 }, { "r3", 3 }, { "r4", 4 },
+	{ "r5", 5 }, { "r6", 6 }, { "r7", 7 }, { "r8", 8 }, { "r9", 9 },
+	{ "r10", 10 }, { "r11", 11 }, { "r12", 12 }, { "r13", 13 }, { "r14", 14 },
+	{ "r15", 15 }, { "r16", 16 }, { "r17", 17 }, { "r18", 18 }, { "r19", 19 },
+	{ "r20", 20 }, { "r21", 21 }, { "r22", 22 }, { "r23", 23 }, { "r24", 24 },
+	{ "r25", 25 }, { "r26", 26 }, { "r27", 27 }, { "r27", 27 }, { "r28", 28 },
+	{ "r29", 29 }, { "r30", 30 }, { "r31", 31 },
+};
+
+int get_arch_regnum(const char *name)
+{
+	unsigned int i;
+
+	if (*name != 'r')
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(powerpc_regidx_table); i++)
+		if (!strcmp(powerpc_regidx_table[i].name, name))
+			return powerpc_regidx_table[i].idx;
+
+	return -ENOENT;
+}
-- 
2.43.0

Re: [PATCH 1/3] tools/perf/arch/powerpc: Add load/store in powerpc annotate instructions for data type profling

[Athira Rajeev]

Hi All,

Please ignore this version. I made mistake in cover letter. I am re-posting the correct version now.
Sorry for the confusion

Thanks
Athira

> On 09-Mar-2024, at 11:21 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> Add powerpc instruction nmemonic table to associate load/store
> instructions with move_ops. mov_ops is used to identify mem_type
> to associate instruction with data type and offset. Also initialize
> and allocate arch specific fields for nr_instructions, instructions and
> nr_instructions_allocate.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> .../perf/arch/powerpc/annotate/instructions.c | 66 +++++++++++++++++++
> 1 file changed, 66 insertions(+)
> 
> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c b/tools/perf/arch/powerpc/annotate/instructions.c
> index a3f423c27cae..07af4442be38 100644
> --- a/tools/perf/arch/powerpc/annotate/instructions.c
> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
> @@ -1,6 +1,65 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <linux/compiler.h>
> 
> +/*
> + * powerpc instruction nmemonic table to associate load/store instructions with
> + * move_ops. mov_ops is used to identify mem_type to associate instruction with
> + * data type and offset.
> + */
> +static struct ins powerpc__instructions[] = {
> + { .name = "lbz", .ops = &mov_ops,  },
> + { .name = "lbzx", .ops = &mov_ops,  },
> + { .name = "lbzu", .ops = &mov_ops,  },
> + { .name = "lbzux", .ops = &mov_ops,  },
> + { .name = "lhz", .ops = &mov_ops,  },
> + { .name = "lhzx", .ops = &mov_ops,  },
> + { .name = "lhzu", .ops = &mov_ops,  },
> + { .name = "lhzux", .ops = &mov_ops,  },
> + { .name = "lha", .ops = &mov_ops,  },
> + { .name = "lhax", .ops = &mov_ops,  },
> + { .name = "lhau", .ops = &mov_ops,  },
> + { .name = "lhaux", .ops = &mov_ops,  },
> + { .name = "lwz", .ops = &mov_ops,  },
> + { .name = "lwzx", .ops = &mov_ops,  },
> + { .name = "lwzu", .ops = &mov_ops,  },
> + { .name = "lwzux", .ops = &mov_ops,  },
> + { .name = "lwa", .ops = &mov_ops,  },
> + { .name = "lwax", .ops = &mov_ops,  },
> + { .name = "lwaux", .ops = &mov_ops,  },
> + { .name = "ld", .ops = &mov_ops,  },
> + { .name = "ldx", .ops = &mov_ops,  },
> + { .name = "ldu", .ops = &mov_ops,  },
> + { .name = "ldux", .ops = &mov_ops,  },
> + { .name = "stb", .ops = &mov_ops,  },
> + { .name = "stbx", .ops = &mov_ops,  },
> + { .name = "stbu", .ops = &mov_ops,  },
> + { .name = "stbux", .ops = &mov_ops,  },
> + { .name = "sth", .ops = &mov_ops,  },
> + { .name = "sthx", .ops = &mov_ops,  },
> + { .name = "sthu", .ops = &mov_ops,  },
> + { .name = "sthux", .ops = &mov_ops,  },
> + { .name = "stw", .ops = &mov_ops,  },
> + { .name = "stwx", .ops = &mov_ops,  },
> + { .name = "stwu", .ops = &mov_ops,  },
> + { .name = "stwux", .ops = &mov_ops,  },
> + { .name = "std", .ops = &mov_ops,  },
> + { .name = "stdx", .ops = &mov_ops,  },
> + { .name = "stdu", .ops = &mov_ops,  },
> + { .name = "stdux", .ops = &mov_ops,  },
> + { .name = "lhbrx", .ops = &mov_ops,  },
> + { .name = "sthbrx", .ops = &mov_ops,  },
> + { .name = "lwbrx", .ops = &mov_ops,  },
> + { .name = "stwbrx", .ops = &mov_ops,  },
> + { .name = "ldbrx", .ops = &mov_ops,  },
> + { .name = "stdbrx", .ops = &mov_ops,  },
> + { .name = "lmw", .ops = &mov_ops,  },
> + { .name = "stmw", .ops = &mov_ops,  },
> + { .name = "lswi", .ops = &mov_ops,  },
> + { .name = "lswx", .ops = &mov_ops,  },
> + { .name = "stswi", .ops = &mov_ops,  },
> + { .name = "stswx", .ops = &mov_ops,  },
> +};
> +
> static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, const char *name)
> {
> int i;
> @@ -52,6 +111,13 @@ static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, con
> static int powerpc__annotate_init(struct arch *arch, char *cpuid __maybe_unused)
> {
> if (!arch->initialized) {
> + arch->nr_instructions = ARRAY_SIZE(powerpc__instructions);
> + arch->instructions = calloc(arch->nr_instructions, sizeof(struct ins));
> + if (arch->instructions == NULL)
> + return -ENOMEM;
> +
> + memcpy(arch->instructions, (struct ins *)powerpc__instructions, sizeof(struct ins) * arch->nr_instructions);
> + arch->nr_instructions_allocated = arch->nr_instructions;
> arch->initialized = true;
> arch->associate_instruction_ops = powerpc__associate_instruction_ops;
> arch->objdump.comment_char      = '#';
> -- 
> 2.43.0
> 

[PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Athira Rajeev]

The function get_dwarf_regnum() returns a DWARF register number
from a register name string. This calls arch specific function
get_arch_regnum to return register number for corresponding arch.
Add mappings for register name to register number in powerpc code:
arch/powerpc/util/dwarf-regs.c

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c b/tools/perf/arch/powerpc/util/dwarf-regs.c
index 0c4f4caf53ac..d955e3e577ea 100644
--- a/tools/perf/arch/powerpc/util/dwarf-regs.c
+++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
@@ -98,3 +98,32 @@ int regs_query_register_offset(const char *name)
 			return roff->ptregs_offset;
 	return -EINVAL;
 }
+
+struct dwarf_regs_idx {
+	const char *name;
+	int idx;
+};
+
+static const struct dwarf_regs_idx powerpc_regidx_table[] = {
+	{ "r0", 0 }, { "r1", 1 }, { "r2", 2 }, { "r3", 3 }, { "r4", 4 },
+	{ "r5", 5 }, { "r6", 6 }, { "r7", 7 }, { "r8", 8 }, { "r9", 9 },
+	{ "r10", 10 }, { "r11", 11 }, { "r12", 12 }, { "r13", 13 }, { "r14", 14 },
+	{ "r15", 15 }, { "r16", 16 }, { "r17", 17 }, { "r18", 18 }, { "r19", 19 },
+	{ "r20", 20 }, { "r21", 21 }, { "r22", 22 }, { "r23", 23 }, { "r24", 24 },
+	{ "r25", 25 }, { "r26", 26 }, { "r27", 27 }, { "r27", 27 }, { "r28", 28 },
+	{ "r29", 29 }, { "r30", 30 }, { "r31", 31 },
+};
+
+int get_arch_regnum(const char *name)
+{
+	unsigned int i;
+
+	if (*name != 'r')
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(powerpc_regidx_table); i++)
+		if (!strcmp(powerpc_regidx_table[i].name, name))
+			return powerpc_regidx_table[i].idx;
+
+	return -ENOENT;
+}
-- 
2.43.0

Re: [PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Christophe Leroy]

Le 09/03/2024 à 08:25, Athira Rajeev a écrit :
> The function get_dwarf_regnum() returns a DWARF register number
> from a register name string. This calls arch specific function
> get_arch_regnum to return register number for corresponding arch.
> Add mappings for register name to register number in powerpc code:
> arch/powerpc/util/dwarf-regs.c
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
>   tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c b/tools/perf/arch/powerpc/util/dwarf-regs.c
> index 0c4f4caf53ac..d955e3e577ea 100644
> --- a/tools/perf/arch/powerpc/util/dwarf-regs.c
> +++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
> @@ -98,3 +98,32 @@ int regs_query_register_offset(const char *name)
>   			return roff->ptregs_offset;
>   	return -EINVAL;
>   }
> +
> +struct dwarf_regs_idx {
> +	const char *name;
> +	int idx;
> +};
> +
> +static const struct dwarf_regs_idx powerpc_regidx_table[] = {
> +	{ "r0", 0 }, { "r1", 1 }, { "r2", 2 }, { "r3", 3 }, { "r4", 4 },
> +	{ "r5", 5 }, { "r6", 6 }, { "r7", 7 }, { "r8", 8 }, { "r9", 9 },
> +	{ "r10", 10 }, { "r11", 11 }, { "r12", 12 }, { "r13", 13 }, { "r14", 14 },
> +	{ "r15", 15 }, { "r16", 16 }, { "r17", 17 }, { "r18", 18 }, { "r19", 19 },
> +	{ "r20", 20 }, { "r21", 21 }, { "r22", 22 }, { "r23", 23 }, { "r24", 24 },
> +	{ "r25", 25 }, { "r26", 26 }, { "r27", 27 }, { "r27", 27 }, { "r28", 28 },
> +	{ "r29", 29 }, { "r30", 30 }, { "r31", 31 },
> +};
> +
> +int get_arch_regnum(const char *name)
> +{
> +	unsigned int i;
> +
> +	if (*name != 'r')
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(powerpc_regidx_table); i++)
> +		if (!strcmp(powerpc_regidx_table[i].name, name))
> +			return powerpc_regidx_table[i].idx;

Can you do more simple ?

Something like:

	int n;

	if (*name != 'r')
		return -EINVAL;
	n = atoi(name + 1);
	return n >= 0 && n < 32 ? n : -ENOENT;

> +
> +	return -ENOENT;
> +}

Re: [PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Athira Rajeev]

> On 09-Mar-2024, at 3:24 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 09/03/2024 à 08:25, Athira Rajeev a écrit :
>> The function get_dwarf_regnum() returns a DWARF register number
>> from a register name string. This calls arch specific function
>> get_arch_regnum to return register number for corresponding arch.
>> Add mappings for register name to register number in powerpc code:
>> arch/powerpc/util/dwarf-regs.c
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>>  tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>> 
>> diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c b/tools/perf/arch/powerpc/util/dwarf-regs.c
>> index 0c4f4caf53ac..d955e3e577ea 100644
>> --- a/tools/perf/arch/powerpc/util/dwarf-regs.c
>> +++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
>> @@ -98,3 +98,32 @@ int regs_query_register_offset(const char *name)
>>   return roff->ptregs_offset;
>>   return -EINVAL;
>>  }
>> +
>> +struct dwarf_regs_idx {
>> + const char *name;
>> + int idx;
>> +};
>> +
>> +static const struct dwarf_regs_idx powerpc_regidx_table[] = {
>> + { "r0", 0 }, { "r1", 1 }, { "r2", 2 }, { "r3", 3 }, { "r4", 4 },
>> + { "r5", 5 }, { "r6", 6 }, { "r7", 7 }, { "r8", 8 }, { "r9", 9 },
>> + { "r10", 10 }, { "r11", 11 }, { "r12", 12 }, { "r13", 13 }, { "r14", 14 },
>> + { "r15", 15 }, { "r16", 16 }, { "r17", 17 }, { "r18", 18 }, { "r19", 19 },
>> + { "r20", 20 }, { "r21", 21 }, { "r22", 22 }, { "r23", 23 }, { "r24", 24 },
>> + { "r25", 25 }, { "r26", 26 }, { "r27", 27 }, { "r27", 27 }, { "r28", 28 },
>> + { "r29", 29 }, { "r30", 30 }, { "r31", 31 },
>> +};
>> +
>> +int get_arch_regnum(const char *name)
>> +{
>> + unsigned int i;
>> +
>> + if (*name != 'r')
>> + return -EINVAL;
>> +
>> + for (i = 0; i < ARRAY_SIZE(powerpc_regidx_table); i++)
>> + if (!strcmp(powerpc_regidx_table[i].name, name))
>> + return powerpc_regidx_table[i].idx;
> 
> Can you do more simple ?
> 
> Something like:
> 
> int n;
> 
> if (*name != 'r')
> return -EINVAL;
> n = atoi(name + 1);
> return n >= 0 && n < 32 ? n : -ENOENT;

Hi Christophe,

Thanks for reviewing patch and for the suggestions.

Sure, I will check this approach and address in V2

Thanks
Athira
> 
>> +
>> + return -ENOENT;
>> +}

Re: [PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Tiezhu Yang]

Hi Athira and Namhyung,

On 03/09/2024 03:25 PM, Athira Rajeev wrote:
> The function get_dwarf_regnum() returns a DWARF register number
> from a register name string. This calls arch specific function
> get_arch_regnum to return register number for corresponding arch.
> Add mappings for register name to register number in powerpc code:
> arch/powerpc/util/dwarf-regs.c
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)

I found commit 3eee606757ad ("perf dwarf-regs: Add get_dwarf_regnum()")
for x86, would you be able to share how to test these changes? What is
the difference with and without the patches?

Thanks,
Tiezhu

Re: [PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc

[Namhyung Kim]

Hello,

On Tue, Mar 26, 2024 at 2:35 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Hi Athira and Namhyung,
>
> On 03/09/2024 03:25 PM, Athira Rajeev wrote:
> > The function get_dwarf_regnum() returns a DWARF register number
> > from a register name string. This calls arch specific function
> > get_arch_regnum to return register number for corresponding arch.
> > Add mappings for register name to register number in powerpc code:
> > arch/powerpc/util/dwarf-regs.c
> >
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > ---
> >  tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
>
> I found commit 3eee606757ad ("perf dwarf-regs: Add get_dwarf_regnum()")
> for x86, would you be able to share how to test these changes? What is
> the difference with and without the patches?

Hmm.. I guess it won't work well without the patch.  This is
needed to map register numbers (from objdump) to what
DWARF can understand (IOW they use different numbers).

Thanks,
Namhyung