[PATCH v4 3/4] gas: sframe: Represent .cfi_undefined RA as FRE without offsets

[Indu Bhagat]

On 7/29/25 4:19 AM, Jens Remus wrote:
>>> @@ -164,6 +164,7 @@ sframe_fre_set_ra_track (struct sframe_row_entry *fre, offsetT ra_offset)
>>>    {
>>>      fre->ra_loc = SFRAME_FRE_ELEM_LOC_STACK;
>>>      fre->ra_offset = ra_offset;
>>> +  fre->ra_undefined = false;
>>>      fre->merge_candidate = false;
>>>    }
>>>    @@ -335,6 +336,9 @@ get_fre_base_reg_id (struct sframe_row_entry *sframe_fre)
>>>      unsigned int cfi_insn_cfa_base_reg = sframe_fre->cfa_base_reg;
>>>      unsigned fre_base_reg = SFRAME_BASE_REG_SP;
>>>    +  if (sframe_fre->ra_undefined)
>>> +    return 0;
>>> +
>> This effectively means SFRAME_BASE_REG_FP; return 0 is generally
>> confusing to read (at least for me).
>>
>> How about we return the SFRAME_BASE_REG_SP instead with some code
>> comments if you see fit.
> My intend was to set as much of the FRE info word zero as possible,
> to make it easier for humans to parse SFrame data for debugging
> purposes.  I did not touch mangled_ra_p, as I was unsure whether
> that can also be unconditionally set to zero.  But I guess that
> would be fine?
> 
> I used zero, to denote that this is about setting the field to zero
> instead of a specific value with an assigned meaning, like
> SFRAME_BASE_REG_SP.
> 
> What if I reverted all the changes to the SFrame helpers and did the
> following change to output_sframe_row_entry instead, to make that
> more explicit?
> 
>     /* Create the fre_info using the CFA base register, number of offsets and max
> -     size of offset in this frame row entry.  */
> -  fre_base_reg = get_fre_base_reg_id (sframe_fre);
> -  fre_num_offsets = get_fre_num_offsets (sframe_fre);
> -  fre_offset_size = sframe_get_fre_offset_size (sframe_fre);
> +     size of offset in this frame row entry.  RA undefined is represented
> +     as FRE without any offsets and all FRE info word fields zeroed.  */
> +  if (sframe_fre->ra_undefined)
> +    {
> +      fre_base_reg = 0;
> +      fre_num_offsets = 0;
> +      fre_offset_size = 0;
> +      fre_mangled_ra_p = 0;
> +    }
> +  else
> +    {
> +      fre_base_reg = get_fre_base_reg_id (sframe_fre);
> +      fre_num_offsets = get_fre_num_offsets (sframe_fre);
> +      fre_offset_size = sframe_get_fre_offset_size (sframe_fre);
> +      fre_mangled_ra_p = sframe_fre->mangled_ra_p;
> +    }
>     fre_info = sframe_set_fre_info (fre_base_reg, fre_num_offsets,
> -                                 fre_offset_size, sframe_fre->mangled_ra_p);
> +                                 fre_offset_size, fre_mangled_ra_p);
>     out_one (fre_info);
> 
>     /* Represent RA undefined as FRE without any offsets.  */
> 
> Alternatively:
> 
>    fre_info = 0;

How about we reset the state of cur_fre when handling the case in 
sframe_xlate_do_cfi_undefined () itself. Then this handling in 
output_sframe_row_entry will not be necessary.