[PATCH v3 09/11] s390: Store SFrame CFA offset adjusted and scaled down

[Jens Remus]

Hello Indu,

thank you for the review and feedback!

On 06.07.2025 01:25, Indu Bhagat via Binutils wrote:
> On 6/27/25 4:08 AM, Jens Remus wrote:
>> In SFrame V2 the size of the offsets following an SFrame FRE can be
>> either signed 8-bit, 16-bit, or 32-bit integer, which the largest offset
> 
> Typo: "with" the largest offset

Sure! My bad.

>> determining their size:
>>    1. CFA offset from CFA base register
>>    2. RA (stack save slot) offset from CFA, usually -48 on s390x if saved
>>    3. FP (stack save slot) offset from CFA, usually -72 on s390x if saved
>> The FP and RA offsets from CFA, when FP/RA saved on the stack, usually
>> have fixed values that fit into signed 8-bit SFrame offsets.  Likewise
>> the DWARF register numbers on s390x of general registers (GR; 0-15) and
>> floating-point registers (FPR; 16-31), when FP/RA saved in registers.
>> With that the CFA offset from CFA base register has the greatest impact
>> on the signed SFrame offset size.
>>
>> The s390x ELF ABI defines the stack pointer (SP) to be 8-byte aligned
>> [1] and the CFA as SP at call site + 160 [2].  The CFA offset from CFA
>> base register is therefore always a multiple of 8.
>>
>> On s390x store the SFrame CFA offset from CFA base register scaled down
>> by the s390x-specific CFA alignment factor of 8, in addition to the
>> adjustment by the s390x-specific CFA adjustment of -160, to further
>> improve the use of signed 8-bit SFrame offsets.  This is similar to the
>> DWARF data alignment factor getting factored out from certain offsets
>> stored in DWARF CFI.
>>
>> [1]: s390x ELF ABI, sections "Register Roles" and "Stack Frame
>>       Allocation", https://github.com/IBM/s390x-abi/releases
>> [2]: s390x ELF ABI, commit 4e38ad9c8a88 ("Document the CFA"),
>>       https://github.com/IBM/s390x-abi/commit/4e38ad9c8a88
>>
>> include/
>>     * sframe.h (SFRAME_S390X_CFA_OFFSET_ALIGNMENT_FACTOR): Define
>>     s390x-specific CFA offset alignment factor.
>>     (SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE,
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE): Scale down/up by
>>     SFRAME_S390X_CFA_OFFSET_ALIGNMENT_FACTOR.
>>
>> libsframe/
>>     * doc/sframe-spec.texi (s390x,
>>     SFRAME_S390X_CFA_OFFSET_ALIGNMENT_FACTOR): Document s390x-
>>     specific CFA offset alignment factor.
>>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> 
> One comment below for sframe-spec.texi.
> 
> Other than that, LGTM.
> 
> Reviewed-by: Indu Bhagat <indu.bhagat@oracle.com>

>> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi

>> @@ -146,10 +146,12 @@ SFRAME_CFA_FIXED_RA_INVALID).  Used on s390x as padding offset to represent
>>   FP without RA saved.
>>   @item SFRAME_S390X_CFA_OFFSET_ADJUSTMENT: CFA offset (from CFA base register)
>>   adjustment value.  Used to enable use of 8-bit SFrame offsets on s390x.
>> +@item SFRAME_S390X_CFA_OFFSET_ALIGNMENT_FACTOR: CFA offset alignment factor.
>> +Used to scale down the CFA offset to improve the use of 8-bit SFrame offsets.
>>   @item SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE: Encode CFA offset (i.e. apply
>> -CFA offset adjustment).
>> -@item SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE: Decode CFA offset (i.e. unapply
>> -CFA offset adjustment).
>> +CFA offset adjustment and then scale down by CFA offset alignment factor).
>> +@item SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE: Decode CFA offset (i.e. scale up
>> +by CFA offset alignment factor and then unapply CFA offset adjustment).
> 
> Comma after i.e. : "i.e.,"
> Instead of unapply, perhaps "revert the CFA offset adjustment" may be better.

Ok.

>>   @end itemize
>>   @end itemize

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

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