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 v1 08/11] perf annotate-data: Add invalidate_reg_state() helper for x86
Date: Tue, 10 Feb 2026 18:38:51 -0800 [thread overview]
Message-ID: <aYvruzvtA5lVam24@google.com> (raw)
In-Reply-To: <20260127020617.2804780-9-zli94@ncsu.edu>
On Mon, Jan 26, 2026 at 09:05:01PM -0500, Zecheng Li wrote:
> Add a helper function to consistently invalidate register state instead
> of field assignments. This ensures kind, ok, and copied_from are all
> properly cleared when a register becomes invalid.
>
> The helper sets:
> - kind = TSR_KIND_INVALID
> - ok = false
> - copied_from = -1
>
> Replace all invalidation patterns with calls to this helper. No
> functional change and this removes some incorrect annotations that were
> caused by incomplete invalidation (e.g. a obsolete copied_from from an
> invalidated register).
It doesn't apply cleanly from here. Please rebase onto the current
perf-tools-next. Maybe we want to wait for v7.0-rc1.
Thanks,
Namhyung
>
> Signed-off-by: Zecheng Li <zli94@ncsu.edu>
> ---
> tools/perf/arch/x86/annotate/instructions.c | 29 ++++++++++++---------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index 803f9351a3fb..e033abb0667b 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -209,6 +209,13 @@ static int x86__annotate_init(struct arch *arch, char *cpuid)
> }
>
> #ifdef HAVE_LIBDW_SUPPORT
> +static void invalidate_reg_state(struct type_state_reg *reg)
> +{
> + reg->kind = TSR_KIND_INVALID;
> + reg->ok = false;
> + reg->copied_from = -1;
> +}
> +
> static void update_insn_state_x86(struct type_state *state,
> struct data_loc_info *dloc, Dwarf_Die *cu_die,
> struct disasm_line *dl)
> @@ -240,7 +247,7 @@ static void update_insn_state_x86(struct type_state *state,
> /* Otherwise invalidate caller-saved registers after call */
> for (unsigned i = 0; i < ARRAY_SIZE(state->regs); i++) {
> if (state->regs[i].caller_saved)
> - state->regs[i].ok = false;
> + invalidate_reg_state(&state->regs[i]);
> }
>
> /* Update register with the return type (if any) */
> @@ -369,8 +376,7 @@ static void update_insn_state_x86(struct type_state *state,
> src_tsr = state->regs[sreg];
> tsr = &state->regs[dst->reg1];
>
> - tsr->copied_from = -1;
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
>
> /* Case 1: Based on stack pointer or frame pointer */
> if (sreg == fbreg || sreg == state->stack_reg) {
> @@ -438,8 +444,7 @@ static void update_insn_state_x86(struct type_state *state,
> !strncmp(dl->ins.name, "inc", 3) || !strncmp(dl->ins.name, "dec", 3)) {
> pr_debug_dtp("%s [%x] invalidate reg%d\n",
> dl->ins.name, insn_offset, dst->reg1);
> - state->regs[dst->reg1].ok = false;
> - state->regs[dst->reg1].copied_from = -1;
> + invalidate_reg_state(&state->regs[dst->reg1]);
> return;
> }
>
> @@ -501,7 +506,7 @@ static void update_insn_state_x86(struct type_state *state,
> if (!get_global_var_type(cu_die, dloc, ip, var_addr,
> &offset, &type_die) ||
> !die_get_member_type(&type_die, offset, &type_die)) {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> return;
> }
>
> @@ -529,7 +534,7 @@ static void update_insn_state_x86(struct type_state *state,
>
> if (!has_reg_type(state, src->reg1) ||
> !state->regs[src->reg1].ok) {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> return;
> }
>
> @@ -565,7 +570,7 @@ static void update_insn_state_x86(struct type_state *state,
>
> stack = find_stack_state(state, offset);
> if (stack == NULL) {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> return;
> } else if (!stack->compound) {
> tsr->type = stack->type;
> @@ -580,7 +585,7 @@ static void update_insn_state_x86(struct type_state *state,
> tsr->offset = 0;
> tsr->ok = true;
> } else {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> return;
> }
>
> @@ -633,7 +638,7 @@ static void update_insn_state_x86(struct type_state *state,
> if (!get_global_var_type(cu_die, dloc, ip, addr, &offset,
> &type_die) ||
> !die_get_member_type(&type_die, offset, &type_die)) {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> return;
> }
>
> @@ -684,7 +689,7 @@ static void update_insn_state_x86(struct type_state *state,
> }
> pr_debug_type_name(&tsr->type, tsr->kind);
> } else {
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> }
> }
> /* And then dereference the calculated pointer if it has one */
> @@ -726,7 +731,7 @@ static void update_insn_state_x86(struct type_state *state,
> }
> }
>
> - tsr->ok = false;
> + invalidate_reg_state(tsr);
> }
> }
> /* Case 3. register to memory transfers */
> --
> 2.52.0
>
next prev parent reply other threads:[~2026-02-11 2:38 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-27 2:04 [PATCH v1 00/11] perf tools: Improvements to data type profiler Zecheng Li
2026-01-27 2:04 ` [PATCH v1 01/11] perf dwarf-aux: Skip check_variable for die_find_variable_by_reg Zecheng Li
2026-02-11 2:08 ` Namhyung Kim
2026-01-27 2:04 ` [PATCH v1 02/11] perf dwarf-aux: Add die_get_pointer_type to get pointer types Zecheng Li
2026-02-11 2:17 ` Namhyung Kim
2026-01-27 2:04 ` [PATCH v1 03/11] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
2026-02-11 2:23 ` Namhyung Kim
2026-01-27 2:04 ` [PATCH v1 04/11] perf annotate-data: Improve type comparison from different scopes Zecheng Li
2026-02-11 2:25 ` Namhyung Kim
2026-01-27 2:04 ` [PATCH v1 05/11] perf dwarf-aux: Handle array types in die_get_member_type Zecheng Li
2026-01-27 2:04 ` [PATCH v1 06/11] perf annotate-data: Collect global variables without name Zecheng Li
2026-02-11 2:29 ` Namhyung Kim
2026-01-27 2:05 ` [PATCH v1 07/11] perf annotate-data: Handle global variable access with const register Zecheng Li
2026-02-11 2:37 ` Namhyung Kim
2026-01-27 2:05 ` [PATCH v1 08/11] perf annotate-data: Add invalidate_reg_state() helper for x86 Zecheng Li
2026-02-11 2:38 ` Namhyung Kim [this message]
2026-01-27 2:05 ` [PATCH v1 09/11] perf annotate-data: Invalidate caller-saved regs for all calls Zecheng Li
2026-01-27 2:05 ` [PATCH v1 10/11] perf annotate-data: Use DWARF location ranges to preserve reg state Zecheng Li
2026-01-27 2:05 ` [PATCH v1 11/11] perf dwarf-aux: Collect all variable locations for insn tracking Zecheng Li
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=aYvruzvtA5lVam24@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 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.