All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Zecheng Li <zecheng@google.com>
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>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Xu Liu <xliuprof@google.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 8/9] perf annotate: Improve type comparison from different scopes
Date: Sat, 25 Oct 2025 10:46:31 -0700	[thread overview]
Message-ID: <aP0M95FMF0fB48x9@google.com> (raw)
In-Reply-To: <20251013181607.2745653-9-zecheng@google.com>

On Mon, Oct 13, 2025 at 06:16:05PM +0000, Zecheng Li wrote:
> When comparing types from different scopes, first compare their type
> offsets. A larger offset means the field belongs to an outer
> (enclosing) struct. This helps resolve cases where a pointer is found
> in an inner scope, but a struct containing that pointer exists in an
> outer scope. Previously, is_better_type would prefer the pointer type,
> but the struct type is actually more complete and should be chosen.

Wouldn't the size check be sufficient?  I think we need to get read of
the pointer preference.

> 
> Prefer types from outer scopes when is_better_type cannot determine
> a better type. This sometimes helps pick a more complete type.

This code has a loop for the scopes so detecting larger struct would
work well?

Thanks,
Namhyung

> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/annotate-data.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 4b08331b9dd3..4b510eb29a5f 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1629,7 +1629,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die)
>  				pr_debug_dtp("type_offset=%#x\n", type_offset);
>  			}
>  
> -			if (!found || is_better_type(type_die, &mem_die)) {
> +			if (!found || dloc->type_offset < type_offset ||
> +				(dloc->type_offset == type_offset &&
> +				 !is_better_type(&mem_die, type_die))) {
>  				*type_die = mem_die;
>  				dloc->type_offset = type_offset;
>  				found = true;
> -- 
> 2.51.0.788.g6d19910ace-goog
> 

  reply	other threads:[~2025-10-25 17:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 18:15 [PATCH v4 0/9] perf tools: Some improvements on data type profiler Zecheng Li
2025-10-13 18:15 ` [PATCH v4 1/9] perf annotate: Skip annotating data types to lea instructions Zecheng Li
2025-10-14 18:40   ` Arnaldo Carvalho de Melo
2025-10-20  4:29     ` Namhyung Kim
2025-10-13 18:15 ` [PATCH v4 2/9] perf annotate: Track address registers via TSR_KIND_POINTER Zecheng Li
2025-10-13 18:16 ` [PATCH v4 3/9] perf annotate: Track arithmetic instructions on pointers Zecheng Li
2025-10-13 18:16 ` [PATCH v4 4/9] perf annotate: Save pointer offset in stack state Zecheng Li
2025-10-13 18:16 ` [PATCH v4 5/9] perf annotate: Invalidate register states for untracked instructions Zecheng Li
2025-10-13 18:16 ` [PATCH v4 6/9] perf dwarf-aux: Skip check_variable for die_find_variable_by_reg Zecheng Li
2025-10-13 18:16 ` [PATCH v4 7/9] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
2025-10-25 17:40   ` Namhyung Kim
2025-10-13 18:16 ` [PATCH v4 8/9] perf annotate: Improve type comparison from different scopes Zecheng Li
2025-10-25 17:46   ` Namhyung Kim [this message]
2025-10-13 18:16 ` [PATCH v4 9/9] perf dwarf-aux: Support DW_OP_piece expressions Zecheng Li
2025-10-25 17:57   ` Namhyung Kim
2025-10-22  0:41 ` [PATCH v4 0/9] perf tools: Some improvements on 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=aP0M95FMF0fB48x9@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=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=xliuprof@google.com \
    --cc=zecheng@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.