Linux Perf Users
 help / color / mirror / Atom feed
From: Shuai Xue <xueshuai@linux.alibaba.com>
To: Tengda Wu <wutengda@huaweicloud.com>,
	Namhyung Kim <namhyung@kernel.org>,
	james.clark@linaro.org, Li Huafei <lihuafei1@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	leo.yan@linux.dev, Ian Rogers <irogers@google.com>,
	Kim Phillips <kim.phillips@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Bill Wendling <morbo@google.com>,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Zecheng Li <zli94@ncsu.edu>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking
Date: Tue, 11 Aug 2026 16:37:49 +0800	[thread overview]
Message-ID: <960b490f-42da-4a9e-8734-62f9c3773b2d@linux.alibaba.com> (raw)
In-Reply-To: <20260808122400.2961238-20-wutengda@huaweicloud.com>



On 8/8/26 8:23 PM, Tengda Wu wrote:
> Extend update_insn_state() for arm64 to support register-to-register and
> immediate-to-register 'mov' instructions.
> 
> For register-to-register 'mov' (e.g., mov dreg, sreg), propagate data type
> information from the source register to the destination register.
> 
> For immediate-to-register 'mov' (e.g., mov dreg, #imm), store the immediate
> value in the destination register's imm_value field and set its kind to
> TSR_KIND_CONST, allowing subsequent instructions to resolve it as a
> constant.
> 
> A real-world example is shown below:
> 
>    ffff8000803eebf8 <get_vma_policy>:
>    ffff8000803eec20:  mov  x21, x0          // x0 (struct vm_area_struct*) -> x21
>    ffff8000803eec28:  ldr  x2, [x0, #112]
>    ffff8000803eec2c:  cbz  x2, ffff8000803eec94 <get_vma_policy+0x9c>
> * ffff8000803eec94:  ldr  x0, [x21, #152]
> 
> Before this commit, the type of x21 was unknown, causing the subsequent
> inference to fail:
> 
>    var [0] reg0 offset 0 type='struct vm_area_struct*' size=0x8
>    chk [9c] reg21 offset=0x98 ok=0 kind=0 cfa : no type information
>    final result: no type information
> 
> After this commit, the type of x21 is correctly inferred as 'vm_area_struct':
> 
>    var [0] reg0 offset 0 type='struct vm_area_struct*' size=0x8
>    mov [28] reg0 -> reg21 type='struct vm_area_struct*' size=0x8
>    chk [9c] reg21 offset=0x98 ok=1 kind=1 (struct vm_area_struct*) : Good!
>    found by insn track: 0x98(reg21) type-offset=0x98
>    final result:  type='struct vm_area_struct' size=0xb0
> 
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
>   .../perf/util/annotate-arch/annotate-arm64.c  | 53 ++++++++++++++++++-
>   1 file changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 6e09e9707256..7b780bad8c07 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   #include <linux/compiler.h>
>   #include <errno.h>
> +#include <inttypes.h>
>   #include <stdlib.h>
>   #include <string.h>
>   #include <linux/ctype.h>
> @@ -484,6 +485,7 @@ static int propagate_load_reg_state(struct type_state *state,
>   		tsr->type = type_die;
>   		tsr->kind = TSR_KIND_TYPE;
>   		tsr->offset = 0;
> +		tsr->imm_value = 0;
>   		tsr->ok = true;
>   
>   		if (src->multi_regs) {
> @@ -641,6 +643,51 @@ static void update_store_insn_state(struct type_state *state,
>   	adjust_reg_index_state(state, dst, insn_name, dl->al.offset);
>   }
>   
> +static void update_mov_insn_state(struct type_state *state,
> +				  struct disasm_line *dl,
> +				  struct annotated_op_loc *src,
> +				  struct annotated_op_loc *dst)
> +{
> +	struct type_state_reg *tsr;
> +	u32 insn_offset = dl->al.offset;
> +	int sreg = src->reg1;
> +	int dreg = dst->reg1;
> +
> +	if (!has_reg_type(state, dreg))
> +		return;
> +
> +	tsr = &state->regs[dreg];
> +	tsr->copied_from = -1;
> +
> +	if (src->imm) {
> +		tsr->kind = TSR_KIND_CONST;
> +		tsr->imm_value = src->offset;
> +		tsr->offset = 0;
> +		tsr->ok = true;

This overwrites only part of the register state: if the destination
previously held a variable with an active DWARF lifetime, the stale
lifetime_active/lifetime_end survive, and the call-invalidation
exemption then protects a plain constant across calls. Same gap as
the return-type block discussed earlier - clearing the register state
(invalidate_reg_state()) before filling it in would cover both.


> +
> +		pr_debug_dtp("mov [%x] imm=%#"PRIx64" -> reg%d\n",
> +			     insn_offset, tsr->imm_value, dreg);
> +		return;
> +	}
> +
> +	if (!has_reg_type(state, sreg) || !state->regs[sreg].ok) {
> +		invalidate_reg_state(tsr);
> +		return;
> +	}
> +
> +	tsr->type = state->regs[sreg].type;
> +	tsr->kind = state->regs[sreg].kind;
> +	tsr->imm_value = state->regs[sreg].imm_value;
> +	tsr->offset = state->regs[sreg].offset;
> +	tsr->ok = state->regs[sreg].ok;

onversely, the register-to-register path drops the lifetime fields
that the x86 handler explicitly copies. Not copying is arguably more
correct - the DWARF location range describes the variable living in
the source register - but it also means a value moved into a
callee-saved register gets invalidated by the first call, losing the
tracking. Is the omission intentional? If so, a comment would help;
either way the two archs should probably converge on one behaviour.

> +
> +	if (tsr->kind == TSR_KIND_TYPE || tsr->kind == TSR_KIND_POINTER)
> +		tsr->copied_from = sreg;
> +
> +	pr_debug_dtp("mov [%x] reg%d -> reg%d", insn_offset, sreg, dreg);
> +	pr_debug_type_name(&tsr->type, tsr->kind);
> +}
> +
>   static void update_insn_state_arm64(struct type_state *state,
>   				    struct data_loc_info *dloc, Dwarf_Die *cu_die,
>   				    struct disasm_line *dl)
> @@ -703,6 +750,7 @@ static void update_insn_state_arm64(struct type_state *state,
>   	 * prevent stale type info from propagating to subsequent instructions.
>   	 */
>   	if (has_reg_type(state, dst->reg1) &&
> +	    strcmp(dl->ins.name, "mov") &&
>   	    strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {

Comparison and test instructions get killed by this: for cmp x5, x6
the first operand ends up as the target (reg1 = x5), and since cmp
matches none of the exemptions above, a read-only register is
invalidated. Same for cmn and tst, and for the tested register of
cbz/cbnz/tbz/tbnz. Since comparisons sit between the tracked load and
the sampled access in almost every hot path, this silently drops a
lot of type resolution.

The x86 side avoids this by using a whitelist of instructions that
actually clobber pointer registers; here an exemption for the
read-only compares/tests would do:

     if (has_reg_type(state, dst->reg1) && !dst->mem_ref &&
         strcmp(dl->ins.name, "mov") && strcmp(dl->ins.name, "cmp") &&
         strcmp(dl->ins.name, "cmn") && strcmp(dl->ins.name, "tst") &&
         ...)

Thanks,
Shuai

  parent reply	other threads:[~2026-08-11  8:38 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
2026-08-08 12:23 ` [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump Tengda Wu
2026-08-10 13:08   ` Shuai Xue
2026-08-11  2:27     ` Tengda Wu
2026-08-11  3:25       ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction " Tengda Wu
2026-08-08 13:03   ` sashiko-bot
2026-08-11  2:37     ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions Tengda Wu
2026-08-08 13:05   ` sashiko-bot
2026-08-11  3:19     ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions Tengda Wu
2026-08-08 13:07   ` sashiko-bot
2026-08-11  7:07     ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64 Tengda Wu
2026-08-08 13:12   ` sashiko-bot
2026-08-11  7:18     ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() " Tengda Wu
2026-08-08 13:07   ` sashiko-bot
2026-08-11  8:10     ` Tengda Wu
2026-08-11  6:33   ` Shuai Xue
2026-08-11  8:16     ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing Tengda Wu
2026-08-08 13:11   ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 08/23] perf annotate-arm64: Implement extract_op_location() callback Tengda Wu
2026-08-11  6:50   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling Tengda Wu
2026-08-10  6:57   ` Adrian Hunter
2026-08-08 12:23 ` [PATCH v4 10/23] perf arm-spe: Set default synthesized event period to 1 Tengda Wu
2026-08-08 12:23 ` [PATCH v4 11/23] perf annotate-data: Extract invalidate_reg_state() as a common helper Tengda Wu
2026-08-11  7:09   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support Tengda Wu
2026-08-08 13:22   ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions Tengda Wu
2026-08-08 13:05   ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking Tengda Wu
2026-08-08 13:08   ` sashiko-bot
2026-08-11  7:36   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 15/23] perf annotate-arm64: Support store " Tengda Wu
2026-08-08 13:11   ` sashiko-bot
2026-08-11  8:00   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 Tengda Wu
2026-08-08 13:17   ` sashiko-bot
2026-08-11  8:10   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 17/23] perf annotate-data: Track imm_value for stack variables Tengda Wu
2026-08-11  8:16   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking Tengda Wu
2026-08-08 13:25   ` sashiko-bot
2026-08-11  8:24   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking Tengda Wu
2026-08-08 13:20   ` sashiko-bot
2026-08-11  8:37   ` Shuai Xue [this message]
2026-08-08 12:23 ` [PATCH v4 20/23] perf annotate-arm64: Support 'add' " Tengda Wu
2026-08-08 13:14   ` sashiko-bot
2026-08-11  8:45   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 21/23] perf annotate-arm64: Support 'adrp' instruction to track global variables Tengda Wu
2026-08-11  8:50   ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking Tengda Wu
2026-08-08 13:18   ` sashiko-bot
2026-08-08 12:24 ` [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer Tengda Wu
2026-08-08 13:20   ` sashiko-bot

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=960b490f-42da-4a9e-8734-62f9c3773b2d@linux.alibaba.com \
    --to=xueshuai@linux.alibaba.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=kim.phillips@arm.com \
    --cc=leo.yan@linux.dev \
    --cc=lihuafei1@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=morbo@google.com \
    --cc=namhyung@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=peterz@infradead.org \
    --cc=wutengda@huaweicloud.com \
    --cc=zli94@ncsu.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox