Re: [PATCH v4 08/23] perf annotate-arm64: Implement extract_op_location() callback

On 8/8/26 8:23 PM, Tengda Wu wrote:
> Implement the extract_op_location() callback for the arm64 architecture
> to handle its specific assembly syntax and addressing modes.
> 
> The extractor handles:
> 1. Standalone immediate operands (e.g., #0x10).
> 2. Memory references with diverse addressing modes:
>     - Signed offset: [base{, #imm|reg}]
>     - Pre-index: [base, #imm]!
>     - Post-index: [base], #imm|reg
> 3. Multi-register operands and primary/secondary register extraction.
> 
> With this callback in place, 'perf annotate' can now resolve memory
> locations and register types on arm64, marking the initial enablement
> of data type profiling for this architecture.
> 
> Signed-off-by: Tengda Wu <[email protected]>
> ---
>   .../perf/util/annotate-arch/annotate-arm64.c  | 67 +++++++++++++++++++
>   tools/perf/util/annotate.h                    | 10 +++
>   2 files changed, 77 insertions(+)
> 
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 32752a3bc756..6b4f1228afa9 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> @@ -287,6 +287,72 @@ static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch,
>   	return ops;
>   }
>   
> +static int extract_op_location_arm64(const struct arch *arch,
> +				     struct disasm_line *dl __maybe_unused,
> +				     const char *op_str, int op_idx __maybe_unused,
> +				     struct annotated_op_loc *op_loc)
> +{
> +	const char *s = op_str;
> +	char *p = NULL;
> +
> +	if (op_str == NULL)
> +		return 0;
> +
> +	/* Handle standalone immediate operands (e.g., #0x10) */
> +	if (*s == arch->objdump.imm_char) {
> +		op_loc->offset = strtol(s + 1, &p, 0);
> +		if (p && p != s + 1)
> +			op_loc->imm = true;
> +		return 0;
> +	}
> +
> +	/*
> +	 * Handle memory references, identify arm64 specific addressing modes.
> +	 * Reference: Arm Architecture Reference Manual
> +	 *            (DDI 0487), Chapter C1.3.3: Load/store addressing modes.
> +	 */
> +	if (*s == arch->objdump.memory_ref_char) {
> +		op_loc->mem_ref = true;
> +
> +		p = strchr(s, ']');
> +		if (p == NULL)
> +			return -1;
> +
> +		/* Pre-index: [base, #imm]! */
> +		if (p[1] == '!')
> +			op_loc->addr_mode = PERF_ADDR_MODE_PRE_INDEX;
> +		/* Post-index: [base], #imm|reg */
> +		else if (p[1] == ',' &&
> +			 (strchr(p + 1, arch->objdump.imm_char) ||
> +			  arm64__is_reg(skip_spaces(p + 2))))
> +			op_loc->addr_mode = PERF_ADDR_MODE_POST_INDEX;
> +		/* Signed offset: [base{, #imm|reg}] */
> +		else
> +			op_loc->addr_mode = PERF_ADDR_MODE_SIGNED_OFFSET;
> +
> +		s++;
> +	}
> +
> +	/* Extract the primary register */
> +	op_loc->reg1 = arch__dwarf_regnum(arch, s);
> +	if (op_loc->reg1 == -1)
> +		return -1;
> +
> +	/* Move to the next symbol of the operand, if any */
> +	s = strchr(s, ',');
> +	if (s == NULL)
> +		return 0;
> +	s = skip_spaces(s + 1);
> +
> +	/* Extract secondary register or immediate offset */
> +	if (op_loc->multi_regs)
> +		op_loc->reg2 = arch__dwarf_regnum(arch, s);
> +	else if (*s == arch->objdump.imm_char)
> +		op_loc->offset = strtol(s + 1, &p, 0);
> +
> +	return 0;
> +}
> +
>   const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
>   				   const char *cpuid __maybe_unused)
>   {
> @@ -305,6 +371,7 @@ const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
>   	arch->objdump.memory_ref_char	  = '[';
>   	arch->objdump.imm_char		  = '#';
>   	arch->associate_instruction_ops   = arm64__associate_instruction_ops;
> +	arch->extract_op_location	  = extract_op_location_arm64;
>   
>   	/* bl, blr */
>   	err = regcomp(&arm->call_insn, "^blr?$", REG_EXTENDED);
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 11b6e4780c02..a44c933f55f5 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -494,12 +494,21 @@ int annotate_check_args(void);
>   
>   int arch__dwarf_regnum(const struct arch *arch, const char *str);
>   
> +enum annotated_addr_mode {
> +	PERF_ADDR_MODE_NONE = 0,
> +
> +	PERF_ADDR_MODE_SIGNED_OFFSET,
> +	PERF_ADDR_MODE_PRE_INDEX,
> +	PERF_ADDR_MODE_POST_INDEX,
> +};
> +
>   /**
>    * struct annotated_op_loc - Location info of instruction operand
>    * @reg1: First register in the operand
>    * @reg2: Second register in the operand
>    * @offset: Memory access offset in the operand
>    * @segment: Segment selector register
> + * @addr_mode: Addressing mode, only valid if @mem_ref is true
>    * @mem_ref: Whether the operand accesses memory
>    * @multi_regs: Whether the second register is used
>    * @imm: Whether the operand is an immediate value (in offset)
> @@ -509,6 +518,7 @@ struct annotated_op_loc {
>   	int reg2;
>   	int offset;
>   	u8 segment;
> +	u8 addr_mode;
>   	bool mem_ref;
>   	bool multi_regs;
>   	bool imm;

I've checked the addressing mode classification against DDI 0487
C1.3.3 and it looks right, including the tricky cases:

   - [x1, w0, sxtw #3] correctly falls through to SIGNED_OFFSET
     (no '#' after ']' and 'sxtw' is not a register);
   - post-index detection handles both immediate and register
     writeback forms;
   - the downstream use is consistent too: post-index accesses happen
     at the old base, so get_reg_index_offset() returning 0 there is
     correct.

Reviewed-by: Shuai Xue <[email protected]>

Thanks.
Shuai