[V2,7/9] tools/perf: Update instruction tracking with add instruction

Update instruction tracking with add instruction. Apart from "mr"
instruction, the register state is carried on by other insns, ie,
"add, addi, addis". Since these are not memory instructions and doesn't
fall in the range of (32 to 63), add these as part of nmemonic table.
For now, add* instructions are added. There is possibility of getting
more added here. But to extract regs, still the binary code will be
used. So associate this with "load_store_ops" itself and no other
changes required.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 .../perf/arch/powerpc/annotate/instructions.c | 21 +++++++++++++++++++
 tools/perf/util/disasm.c                      |  1 +
 2 files changed, 22 insertions(+)

Le 06/05/2024 à 14:19, Athira Rajeev a écrit :
> Update instruction tracking with add instruction. Apart from "mr"
> instruction, the register state is carried on by other insns, ie,
> "add, addi, addis". Since these are not memory instructions and doesn't
> fall in the range of (32 to 63), add these as part of nmemonic table.
> For now, add* instructions are added. There is possibility of getting
> more added here. But to extract regs, still the binary code will be
> used. So associate this with "load_store_ops" itself and no other
> changes required.

Looks fragile.

How do you handle addc, adde, addic ?
And also addme, addze, which only have two operands instead of 3 ?

> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
>   .../perf/arch/powerpc/annotate/instructions.c | 21 +++++++++++++++++++
>   tools/perf/util/disasm.c                      |  1 +
>   2 files changed, 22 insertions(+)
> 
> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c b/tools/perf/arch/powerpc/annotate/instructions.c
> index cce7023951fe..1f35d8a65bb4 100644
> --- a/tools/perf/arch/powerpc/annotate/instructions.c
> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
> @@ -1,6 +1,17 @@
>   // 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 add/mr to do instruction tracking.
> + */
> +static struct ins powerpc__instructions[] = {
> +	{ .name = "mr",         .ops = &load_store_ops,  },
> +	{ .name = "addi",       .ops = &load_store_ops,   },
> +	{ .name = "addis",      .ops = &load_store_ops,  },
> +	{ .name = "add",        .ops = &load_store_ops,  },
> +};
> +
>   static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, const char *name)
>   {
>   	int i;
> @@ -75,6 +86,9 @@ static void update_insn_state_powerpc(struct type_state *state,
>   	if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
>   		return;
>   
> +	if (!strncmp(dl->ins.name, "add", 3))
> +		goto regs_check;
> +
>   	if (strncmp(dl->ins.name, "mr", 2))
>   		return;
>   
> @@ -85,6 +99,7 @@ static void update_insn_state_powerpc(struct type_state *state,
>   		dst->reg1 = src_reg;
>   	}
>   
> +regs_check:
>   	if (!has_reg_type(state, dst->reg1))
>   		return;
>   
> @@ -115,6 +130,12 @@ static void update_insn_state_powerpc(struct type_state *state __maybe_unused, s
>   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)
> +			return -ENOMEM;
> +		memcpy(arch->instructions, 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      = '#';
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index ac6b8b8da38a..32cf506a9010 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -36,6 +36,7 @@ static struct ins_ops mov_ops;
>   static struct ins_ops nop_ops;
>   static struct ins_ops lock_ops;
>   static struct ins_ops ret_ops;
> +static struct ins_ops load_store_ops;
>   
>   static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
>   			   struct ins_operands *ops, int max_ins_name);

> On 7 May 2024, at 3:28 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 06/05/2024 à 14:19, Athira Rajeev a écrit :
>> Update instruction tracking with add instruction. Apart from "mr"
>> instruction, the register state is carried on by other insns, ie,
>> "add, addi, addis". Since these are not memory instructions and doesn't
>> fall in the range of (32 to 63), add these as part of nmemonic table.
>> For now, add* instructions are added. There is possibility of getting
>> more added here. But to extract regs, still the binary code will be
>> used. So associate this with "load_store_ops" itself and no other
>> changes required.
> 
> Looks fragile.
> 
> How do you handle addc, adde, addic ?
> And also addme, addze, which only have two operands instead of 3 ?

Hi Christophe,

Thanks for pointing these cases. Will address these cases in next version

Thanks
Athira
> 
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>>  .../perf/arch/powerpc/annotate/instructions.c | 21 +++++++++++++++++++
>>  tools/perf/util/disasm.c                      |  1 +
>>  2 files changed, 22 insertions(+)
>> 
>> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c b/tools/perf/arch/powerpc/annotate/instructions.c
>> index cce7023951fe..1f35d8a65bb4 100644
>> --- a/tools/perf/arch/powerpc/annotate/instructions.c
>> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
>> @@ -1,6 +1,17 @@
>>  // 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 add/mr to do instruction tracking.
>> + */
>> +static struct ins powerpc__instructions[] = {
>> + { .name = "mr",         .ops = &load_store_ops,  },
>> + { .name = "addi",       .ops = &load_store_ops,   },
>> + { .name = "addis",      .ops = &load_store_ops,  },
>> + { .name = "add",        .ops = &load_store_ops,  },
>> +};
>> +
>>  static struct ins_ops *powerpc__associate_instruction_ops(struct arch *arch, const char *name)
>>  {
>>   int i;
>> @@ -75,6 +86,9 @@ static void update_insn_state_powerpc(struct type_state *state,
>>   if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
>>   return;
>> 
>> + if (!strncmp(dl->ins.name, "add", 3))
>> + goto regs_check;
>> +
>>   if (strncmp(dl->ins.name, "mr", 2))
>>   return;
>> 
>> @@ -85,6 +99,7 @@ static void update_insn_state_powerpc(struct type_state *state,
>>   dst->reg1 = src_reg;
>>   }
>> 
>> +regs_check:
>>   if (!has_reg_type(state, dst->reg1))
>>   return;
>> 
>> @@ -115,6 +130,12 @@ static void update_insn_state_powerpc(struct type_state *state __maybe_unused, s
>>  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)
>> + return -ENOMEM;
>> + memcpy(arch->instructions, 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      = '#';
>> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
>> index ac6b8b8da38a..32cf506a9010 100644
>> --- a/tools/perf/util/disasm.c
>> +++ b/tools/perf/util/disasm.c
>> @@ -36,6 +36,7 @@ static struct ins_ops mov_ops;
>>  static struct ins_ops nop_ops;
>>  static struct ins_ops lock_ops;
>>  static struct ins_ops ret_ops;
>> +static struct ins_ops load_store_ops;
>> 
>>  static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
>>     struct ins_operands *ops, int max_ins_name);