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

[Jens Remus]

On 1/28/2026 9:13 AM, Indu Bhagat wrote:
> On 1/27/26 12:28 AM, Jens Remus wrote:
>> 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)

CTRL is also fine (matching the US keyboard labeling for the control
key).  Why did you use the REG prefix?  DEREF_P=1 also applies if
REG_P=0, so that REGNUM is not used.  The REG prefix makes one assume
that any REG-prefixed things only apply if REG_P=1.  I also prefer
REGNUM instead of REG_NUM.

> 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?

I would prefer to get rid of the REG prefix.

I guess you would like to enable future CTRLWORDs for other purposes?
If this is a Register Control Word, what about:

  REGCTRL_REGNUM
  REGCTRL_DEREF_P
  REGCTRL_REG_P

Or RCW for Register Control Word:

  RCW_REGNUM
  RCW_DEREF_P
  RCW_REG_P

>>> 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..)

I did not think about that.  As you prefer.  I have no strong opinion.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/