Re: [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Aug 13, 2026 at 10:54:00PM +0800, Tengda Wu wrote:
> 
> 
> On 2026/8/11 15:36, Shuai Xue wrote:
> > 
> > 
> > On 8/8/26 8:23 PM, Tengda Wu wrote:
> >> Extend update_insn_state_arm64() to handle LDR instructions, tracking
> >> register state changes when data is loaded from memory to registers.
> >>
> >> The implementation handles the three primary arm64 addressing modes:
> >> 1. Signed offset: [base, #imm|reg]
> >> 2. Pre-index: [base, #imm]!
> >> 3. Post-index: [base], #imm
> >>
> >> Before updating, check the addressing mode via get_reg_index_offset() to
> >> obtain the actual source's reg_offset, and then propagate the type.
> >>
> >> Since a load instruction may have two destination registers (in ldp cases),
> >> introduce propagate_load_reg_state() to propagate the type for a specified
> >> destination register using a given reg_offset. The respective reg_offset
> >> values for the two registers are as follows:
> >>
> >>    dst->reg1: reg_offset = get_reg_index_offset()
> >>    dst->reg2: reg_offset = get_reg_index_offset() + reg_size(dst->reg1)
> >>
> >> Finally, handle the side effects of pre-index and post-index addressing
> >> via adjust_reg_index_state().
> >>
> >> A real-world example is shown below:
> >>
> >>    ffff80008011f5b0 <pick_task_stop>:
> >>    ffff80008011f5b8:  ldr  x0, [x0, #2712] // x0: struct rq* -> task_struct*
> >> * ffff80008011f5c0:  ldr  w1, [x0, #104]
> >>
> >> Before this commit, the type of x0 was incorrectly inferred as 'struct rq':
> >>
> >>    find data type for 0x68(reg0) at pick_task_stop+0x10
> >>    var [8] reg0 offset 0 type='struct rq*'
> >>    chk [10] reg0 offset=0x68 ok=1 kind=1 (struct rq*) : Good!
> >>    final result:  type='struct rq'
> >>
> >> After this commit, the type of x0 is correctly inferred as 'struct task_struct':
> >>
> >>    find data type for 0x68(reg0) at pick_task_stop+0x10
> >>    var [8] reg0 offset 0 type='struct rq*'
> >>    ldr [8] 0xa98(reg0) -> reg0 type='struct task_struct*'
> >>    chk [10] reg0 offset=0x68 ok=1 kind=1 (struct task_struct*) : Good!
> >>    final result: type='struct task_struct'
> >>
> >> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> >> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> >> ---
> >>   .../perf/util/annotate-arch/annotate-arm64.c  | 148 +++++++++++++++++-
> >>   1 file changed, 147 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> >> index acff14ca01e0..6557c0ad11b2 100644
> >> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> >> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> >> @@ -358,11 +358,152 @@ static int extract_op_location_arm64(const struct arch *arch,
> >>   }
> >>     #ifdef HAVE_LIBDW_SUPPORT
> >> +static int arm64__reg_size(const char *reg)
> >> +{
> >> +    if (!reg || !*reg || !arm64__is_reg(reg))
> >> +        return -1;
> > 
> > Since arm64__is_reg() rejects xzr and SIMD registers, something like
> > ldp xzr, x19, [sp] ends up with multi_regs = false and x19 never
> > invalidated or tracked. Admittedly a corner case - but maybe worth
> > handling if extending the register set is cheap.
> > 
> 
> Agreed. But I'd propose that we first support recognizing xzr/wzr only,
> and leave SIMD registers for a future extension when we properly add SIMD support.
> 
> There shouldn't be a case where SIMD registers and general-purpose registers
> appear together within the same operands, right? If so, then skipping SIMD
> support for now should be fine.

I agree we should focus on GP registers and skip SIMD for now.

Thanks,
Namhyung