[PATCH,V2 0/5] sframe: use data word instead of offset

[Indu Bhagat]

On 1/27/26 12:28 AM, Jens Remus wrote:
> Hello Indu!
> 
> On 1/26/2026 6:39 PM, Jens Remus wrote:
>> On 1/23/2026 11:35 AM, Indu Bhagat via Binutils wrote:
>>> This version is rebased on latest master and addresses Jens' reviews.
>>>
>>> V1 posting: https://inbox.sourceware.org/binutils/20260120102824.2522876-1-indu.bhagat@oracle.com/
>>>
>>> I will take one more look at these before committing.
>>
>> I just realized that there are a few leftovers in include/sframe.h:
>>
>> #define SFRAME_V3_FLEX_FDE_OFFSET_REG_NUM(data)       ((data) >> 3)
>> #define SFRAME_V3_FLEX_FDE_OFFSET_REG_DEREF_P(data)   (((data) >> 1) & 0x1)
>> #define SFRAME_V3_FLEX_FDE_OFFSET_REG_P(data)         ((data) & 0x1)
>>
>> What about the following?
>>
>> #define SFRAME_V3_FLEX_FDE_DATAWORD_REGNUM(data)      ((data) >> 3)
>> #define SFRAME_V3_FLEX_FDE_DATAWORD_DEREF_P(data)     (((data) >> 1) & 0x1)
>> #define SFRAME_V3_FLEX_FDE_DATAWORD_REG_P(data)       ((data) & 0x1)
> 
> Maybe better CTLWORD instead of DATAWORD?
> 
> #define SFRAME_V3_FLEX_FDE_CTLWORD_REGNUM(data)       ((data) >> 3)
> #define SFRAME_V3_FLEX_FDE_CTLWORD_DEREF_P(data)      (((data) >> 1) & 0x1)
> #define SFRAME_V3_FLEX_FDE_CTLWORD_REG_P(data)        ((data) & 0x1)
> 

Sure, but how about "CTRLWORD" instead:

#define SFRAME_V3_FLEX_FDE_CTRLWORD_REG_NUM(data)       ((data) >> 3)
#define SFRAME_V3_FLEX_FDE_CTRLWORD_REG_DEREF_P(data)   (((data) >> 1) & 
0x1)
#define SFRAME_V3_FLEX_FDE_CTRLWORD_REG_P(data)         ((data) & 0x1)

If we are doing the above, perhaps we also include:

-#define SFRAME_V3_FLEX_FDE_REG_ENCODE(reg, deref_p, reg_p)  \
+#define SFRAME_V3_FLEX_FDE_CTRLWORD_REG_ENCODE(reg, deref_p, reg_p)  \
    ((((reg) << 0x3) | (0 << 0x2) | (((deref_p) & 0x1) << 0x1) | 
((reg_p) & 0x1)))

WDYT?

>> Also a V3 version of FRE_RA_UNDEFINED_P() is missing.  What about:
>>
>> #define SFRAME_V3_FRE_RA_UNDEFINED_P(data)        (SFRAME_V2_FRE_RA_UNDEFINED_P(data))
> 
> Should we change the terminology to FRE_OUTERMOST_P for outermost frame,
> similar to FRE_SIGNAL_P?
> 
> #define SFRAME_V3_FRE_OUTERMOST_P(data)           (SFRAME_V2_FRE_RA_UNDEFINED_P(data))
> 

If it is just about defining this macro in sframe.h, I think that will 
be OK (as it serves documentation purposes).

But then we have libsframe API sframe_fre_get_ra_undefined_p too: if we 
intend to change libsframe/sframe.c and libsframe/libsframe.ver, I'm not 
sure if its worth it at this time (because ideally we keep 2_46 branch 
sync'd up for libsframe diffs..)