Re: [PATCH v4 17/23] perf annotate-data: Track imm_value for stack variables

On 8/8/26 8:23 PM, Tengda Wu wrote:
> Currently, imm_value are tracked in registers but dropped when values

Nit: s/imm_value are tracked/imm_value is tracked/ in the changelog.

> are saved to or loaded from stack variables during type profiling.
> If a register of type TSR_KIND_CONST is stored to the stack, its value
> will be lost.
> 
> Add an imm_value field to struct type_state_stack and update
> set_stack_state() and findnew_stack_state() to accept and preserve the
> immediate value when updating stack state.
> 
> Signed-off-by: Tengda Wu <[email protected]>
> ---
>   tools/perf/util/annotate-arch/annotate-x86.c |  8 ++++++--
>   tools/perf/util/annotate-data.c              | 13 +++++++------
>   tools/perf/util/annotate-data.h              |  5 +++--
>   3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index eec3d8ce00b8..995117525e7d 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c
> @@ -591,6 +591,7 @@ static void update_insn_state_x86(struct type_state *state,
>   				tsr->type = stack->type;
>   				tsr->kind = stack->kind;
>   				tsr->offset = stack->ptr_offset;
> +				tsr->imm_value = stack->imm_value;
>   				tsr->ok = true;
>   			} else if (die_get_member_type(&stack->type,
>   						       offset - stack->offset,
> @@ -598,6 +599,7 @@ static void update_insn_state_x86(struct type_state *state,
>   				tsr->type = type_die;
>   				tsr->kind = TSR_KIND_TYPE;
>   				tsr->offset = 0;
> +				tsr->imm_value = 0;
>   				tsr->ok = true;
>   			} else {
>   				invalidate_reg_state(tsr);
> @@ -773,10 +775,12 @@ static void update_insn_state_x86(struct type_state *state,
>   				 */
>   				if (!stack->compound)
>   					set_stack_state(stack, offset, tsr->kind,
> -							&tsr->type, tsr->offset);
> +							&tsr->type, tsr->offset,
> +							tsr->imm_value);
>   			} else {
>   				findnew_stack_state(state, offset, tsr->kind,
> -						    &tsr->type, tsr->offset);
> +						    &tsr->type, tsr->offset,
> +						    tsr->imm_value);
>   			}
>   
>   			if (dst->reg1 == fbreg) {
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index c51b20a7af9b..9dcdc3c7e688 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -590,7 +590,7 @@ struct type_state_stack *find_stack_state(struct type_state *state,
>   }
>   
>   void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> -			    Dwarf_Die *type_die, int ptr_offset)
> +			    Dwarf_Die *type_die, int ptr_offset, u64 imm_value)
>   {
>   	int tag;
>   	Dwarf_Word size;
> @@ -607,8 +607,9 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
>   	stack->offset = offset;
>   	stack->ptr_offset = ptr_offset;
>   	stack->kind = kind;
> +	stack->imm_value = imm_value;
>   
> -	if (kind == TSR_KIND_POINTER) {
> +	if (kind == TSR_KIND_POINTER || kind == TSR_KIND_CONST) {
>   		stack->compound = false;
>   		return;
>   	}
> @@ -629,18 +630,18 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
>   struct type_state_stack *findnew_stack_state(struct type_state *state,
>   						    int offset, u8 kind,
>   						    Dwarf_Die *type_die,
> -						    int ptr_offset)
> +						    int ptr_offset, u64 imm_value)
>   {
>   	struct type_state_stack *stack = find_stack_state(state, offset);
>   
>   	if (stack) {
> -		set_stack_state(stack, offset, kind, type_die, ptr_offset);
> +		set_stack_state(stack, offset, kind, type_die, ptr_offset, imm_value);
>   		return stack;
>   	}
>   
>   	stack = malloc(sizeof(*stack));
>   	if (stack) {
> -		set_stack_state(stack, offset, kind, type_die, ptr_offset);
> +		set_stack_state(stack, offset, kind, type_die, ptr_offset, imm_value);
>   		list_add(&stack->list, &state->stack_vars);
>   	}
>   	return stack;
> @@ -935,7 +936,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
>   				continue;
>   
>   			findnew_stack_state(state, offset, TSR_KIND_TYPE,
> -					    &mem_die, /*ptr_offset=*/0);
> +					    &mem_die, /*ptr_offset=*/0, /*imm_value=*/0);
>   
>   			if (var->reg == state->stack_reg) {
>   				pr_debug_dtp("var [%"PRIx64"] %#x(reg%d)",
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 91b83e94c51b..06fc4dbfb35c 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -199,6 +199,7 @@ struct type_state_stack {
>   	int size;
>   	bool compound;
>   	u8 kind;
> +	u64 imm_value;
>   };
>   
>   /*
> @@ -253,9 +254,9 @@ bool has_reg_type(struct type_state *state, int reg);
>   struct type_state_stack *findnew_stack_state(struct type_state *state,
>   						int offset, u8 kind,
>   						Dwarf_Die *type_die,
> -						int ptr_offset);
> +						int ptr_offset, u64 imm_value);
>   void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> -				Dwarf_Die *type_die, int ptr_offset);
> +				Dwarf_Die *type_die, int ptr_offset, u64 imm_value);
>   struct type_state_stack *find_stack_state(struct type_state *state,
>   						int offset);
>   void invalidate_reg_state(struct type_state_reg *reg);


Reviewed-by: Shuai Xue <[email protected]>

Thanks.
Shuai