public inbox for linux-perf-users@vger.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Zecheng Li <zli94@ncsu.edu>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>,
	xliuprof@google.com, linux-perf-users@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 00/11] perf tools: Improvements to data type profiler
Date: Mon, 16 Mar 2026 11:17:02 -0700	[thread overview]
Message-ID: <abhJHjMHbHILpbmN@google.com> (raw)
In-Reply-To: <20260309175546.916039-1-zli94@ncsu.edu>

Hi Zecheng,

Thanks for working on this continuously!

On Mon, Mar 09, 2026 at 01:55:13PM -0400, Zecheng Li wrote:
> This patch series improves the coverage and correctness of data type
> annotations in the perf tools.
> 
> Here's a breakdown of the patches:
> 
> Patches 1-4 improve variable type matching:
> - Add die_get_pointer_type() to properly handle typedef'd pointer types
> - Preserve typedefs in match_var_offset for consistent type handling
> - Skip redundant check_variable for die_find_variable_by_reg and
>   die_find_variable_by_addr since match_var_offset already performs
>   sufficient checking
> - Improve type comparison when variables are found in different scopes
> 
> Patch 5 handles array types in die_get_member_type, allowing proper
> resolution of struct members that are arrays.
> 
> Patches 6-7 improve global variable handling:
> - Allow collecting global variables without symbol names (DWARF provides
>   the address directly via DW_OP_addr)
> - Handle global variable access when a register holds a const value
>   with negative offset
> 
> Patches 8-9 improve caller-saved register handling:
> - Add invalidate_reg_state() helper for consistent register invalidation
> - Always invalidate caller-saved registers for call instructions per ABI
>   requirements, even when the call target is unknown
> 
> Patches 10-11 use DWARF location ranges to improve type tracking:
> - Track DWARF location lifetime to preserve register state across calls
>   when the debug info indicates the value is still valid
> - Collect all variable location entries instead of just the first one
> 
> Tested with the Linux kernel vmlinux with sampled functions and five
> programs from binutils (as, ld, nm, objdump, readelf) with all
> functions. Coverage rate includes all memory access instructions,
> excluding stack memory accesses.
> 
> `lost` means coverage loss (only including annotated -> none);
> `chg` means type name change. Net coverage gain is new_rate - prev_rate.
> 
>          -------- vmlinux --------   ------- binutils -------
>   Patch  rate     lost     chg       rate     lost     chg
>   ----------------------------------------------------------------
>   base   88.87%   -        -         72.55%   -        -
>   1      88.87%   0%       0%        74.22%   0%       .03%
>   2      88.88%   0%       .05%      74.32%   0%       0%
>   3      88.90%   0%       .69%      74.38%   0%       .31%
>   4      88.90%   0%       .83%      74.38%   0%       .17%
>   5      89.11%   .01%     .08%      74.41%   0%       .01%
>   6      89.12%   0%       .05%      75.33%   0%       1.62%
>   7      89.18%   0%       0%        75.33%   0%       0%
>   8      89.18%   0%       0%        75.30%   .02%     0%
>   9      89.09%   .09%     0%        75.24%   .05%     0%
>   10     89.05%   .08%     .09%      75.53%   .03%     .03%
>   11     89.97%   0%       .10%      76.63%   .26%     .07%

It's interesting that the patch 11 lost some coverage as it merely added
more variables (well the same variables with different ranges).
Have you checked the lost cases and what happened there?

Thanks,
Namhyung

> 
>   Total: vmlinux  88.87% -> 89.97% (+1.10%)
>          binutils 72.55% -> 76.63% (+4.08%)
> 
> Changes in v2:
> - Reorder patches 1-3 to avoid temporary regression: pointer_type and
>   typedef patches now come before skip check_variable (Namhyung)
> - Skip check_variable for both die_find_variable_by_reg and
>   die_find_variable_by_addr paths (Namhyung)
> - Add dso__kernel() guard for const register global variable access
>   to restrict it to kernel DSOs (Namhyung)
> 
> Zecheng Li (11):
>   perf dwarf-aux: Add die_get_pointer_type to get pointer types
>   perf dwarf-aux: Preserve typedefs in match_var_offset
>   perf dwarf-aux: Skip check_variable for variable lookup
>   perf annotate-data: Improve type comparison from different scopes
>   perf dwarf-aux: Handle array types in die_get_member_type
>   perf annotate-data: Collect global variables without name
>   perf annotate-data: Handle global variable access with const register
>   perf annotate-data: Add invalidate_reg_state() helper for x86
>   perf annotate-data: Invalidate caller-saved regs for all calls
>   perf annotate-data: Use DWARF location ranges to preserve reg state
>   perf dwarf-aux: Collect all variable locations for insn tracking
> 
>  tools/perf/util/annotate-arch/annotate-x86.c |  69 ++++++---
>  tools/perf/util/annotate-data.c              | 119 ++++++++-------
>  tools/perf/util/annotate-data.h              |   3 +
>  tools/perf/util/dwarf-aux.c                  | 145 +++++++++++++------
>  tools/perf/util/dwarf-aux.h                  |   9 +-
>  5 files changed, 230 insertions(+), 115 deletions(-)
> 
> -- 
> 2.53.0
> 

  parent reply	other threads:[~2026-03-16 18:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 17:55 [PATCH v2 00/11] perf tools: Improvements to data type profiler Zecheng Li
2026-03-09 17:55 ` [PATCH v2 01/11] perf dwarf-aux: Add die_get_pointer_type to get pointer types Zecheng Li
2026-03-09 17:55 ` [PATCH v2 02/11] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
2026-03-09 17:55 ` [PATCH v2 03/11] perf dwarf-aux: Skip check_variable for variable lookup Zecheng Li
2026-03-09 17:55 ` [PATCH v2 04/11] perf annotate-data: Improve type comparison from different scopes Zecheng Li
2026-03-09 17:55 ` [PATCH v2 05/11] perf dwarf-aux: Handle array types in die_get_member_type Zecheng Li
2026-03-09 17:55 ` [PATCH v2 06/11] perf annotate-data: Collect global variables without name Zecheng Li
2026-03-09 17:55 ` [PATCH v2 07/11] perf annotate-data: Handle global variable access with const register Zecheng Li
2026-03-09 17:55 ` [PATCH v2 08/11] perf annotate-data: Add invalidate_reg_state() helper for x86 Zecheng Li
2026-03-09 17:55 ` [PATCH v2 09/11] perf annotate-data: Invalidate caller-saved regs for all calls Zecheng Li
2026-03-09 17:55 ` [PATCH v2 10/11] perf annotate-data: Use DWARF location ranges to preserve reg state Zecheng Li
2026-03-09 17:55 ` [PATCH v2 11/11] perf dwarf-aux: Collect all variable locations for insn tracking Zecheng Li
2026-03-16 18:17 ` Namhyung Kim [this message]
2026-03-20 18:12 ` [PATCH v2 00/11] perf tools: Improvements to data type profiler Namhyung Kim

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=abhJHjMHbHILpbmN@google.com \
    --to=namhyung@kernel.org \
    --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=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=xliuprof@google.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