[7/8] s390: Store SFrame CFA offset adjusted
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
In SFrame V2 the size of the one to three offsets following a SFrame FDE
can be either signed 8-bit, 16-bit, or 32-bit integer, which the largest
offset 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 [1] defines the CFA as stack pointer (SP) at call
site +160. [2] Therefore the minimum CFA offset from CFA base register
on s390x is 160. This does not fit into a signed 8-bit integer and
therefore effectively prevents any use of signed 8-bit SFrame offsets
on s390x.
For s390x store the CFA offset from CFA base register adjusted by -160
to enable the use of signed 8-bit SFrame offsets.
[1]: s390x ELF ABI, 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_S390_CFA_OFFSET_ADJUSTMENT): Define
s390x-specific CFA offset adjustment.
(SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE,
SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE): New S390-specific macros.
Use SFRAME_S390_CFA_OFFSET_ADJUSTMENT to en-/decode CFA offset.
bfd/
* elf64-s390.c (elf_s390x_sframe_plt_fre): Use
SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE on CFA offset to store it
adjusted and switch to 8-bit offsets.
gas/
* gen-sframe.c (sframe_fre_set_cfa_offset): For S390 use
SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE on CFA offset to store it
adjusted.
(sframe_fre_get_cfa_offset): New helper. For S390 use
SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE on CFA offset to undo its
adjustment.
(sframe_xlate_do_def_cfa_register): Use new helper
sframe_fre_get_cfa_offset.
libsframe/
* sframe.c (sframe_fre_get_cfa_offset): For S390 use
SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE on CFA offset to undo its
adjustment.
* doc/sframe-spec.texi (S390,
SFRAME_S390_CFA_OFFSET_ADJUSTMENT,
SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE,
SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE): Document S390-specific
adjustment of CFA offset.
Suggested-by: Indu Bhagat <indu.bhagat@oracle.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
Notes (jremus):
A test build of Glibc tag 2.41 on s390x libc.so shows a ~8.8% reduction in
.sframe section size due to storing the SFrame CFA offsets adjusted by
-160.
Statistics for libc.so - base (no CFA offset adjustment):
.sframe size: 169,749 bytes
VALUE TOTAL MIN MAX AVG
FDEs: 3652 - - -
FREs/FDE: 15236 1 20 4
Offsets/FDE: 29792 1 38 8
8-bit: 0 0 0 0
16-bit: 29792 1 38 8
32-bit: 0 0 0 0
Offsets/FRE: 29792 1 3 1
8-bit: - 0 0 0
16-bit: - 1 3 1
32-bit: - 0 0 0
O_Padd/FDE: 342 - - 0
8-bit: 0
16-bit: 342
32-bit: 0
Statistics for libc.so - CFA offset adjustment:
.sframe size: 154,757 bytes
VALUE TOTAL MIN MAX AVG
FDEs: 3654 - - -
FREs/FDE: 15238 1 20 4
Offsets/FDE: 29794 1 38 8
8-bit: 14992 1 38 4
16-bit: 14802 0 0 4
32-bit: 0 0 0 0
Offsets/FRE: 29794 2 6 1
8-bit: - 1 3 0
16-bit: - 1 3 0
32-bit: - 0 0 0
O_Padd/FDE: 342 - - 0
8-bit: 283
16-bit: 59
32-bit: 0
bfd/elf64-s390.c | 4 ++--
gas/gen-sframe.c | 18 +++++++++++++++++-
include/sframe.h | 8 ++++++++
libsframe/doc/sframe-spec.texi | 10 +++++++++-
libsframe/sframe.c | 10 ++++++++--
5 files changed, 44 insertions(+), 6 deletions(-)
Comments
On 4/2/25 9:12 AM, Jens Remus wrote:
> In SFrame V2 the size of the one to three offsets following a SFrame FDE
"following an SFrame FRE"
Also, I would avoid saying "of the one to three offsets". The format by
itself does not limit the number of offsets, though for all current
practical usages, we see 1-3 offsets. It is, of course, not recommended
to use the format to encode large number of offsets.
The reason to avoid saying so is that it can be confusing to see that in
git logs.
> can be either signed 8-bit, 16-bit, or 32-bit integer, which the largest
> offset 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 [1] defines the CFA as stack pointer (SP) at call
> site +160. [2] Therefore the minimum CFA offset from CFA base register
> on s390x is 160. This does not fit into a signed 8-bit integer and
> therefore effectively prevents any use of signed 8-bit SFrame offsets
> on s390x.
>
> For s390x store the CFA offset from CFA base register adjusted by -160
> to enable the use of signed 8-bit SFrame offsets.
>
> [1]: s390x ELF ABI, 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_S390_CFA_OFFSET_ADJUSTMENT): Define
> s390x-specific CFA offset adjustment.
> (SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE,
> SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE): New S390-specific macros.
> Use SFRAME_S390_CFA_OFFSET_ADJUSTMENT to en-/decode CFA offset.
>
> bfd/
> * elf64-s390.c (elf_s390x_sframe_plt_fre): Use
> SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE on CFA offset to store it
> adjusted and switch to 8-bit offsets.
>
> gas/
> * gen-sframe.c (sframe_fre_set_cfa_offset): For S390 use
> SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE on CFA offset to store it
> adjusted.
> (sframe_fre_get_cfa_offset): New helper. For S390 use
> SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE on CFA offset to undo its
> adjustment.
> (sframe_xlate_do_def_cfa_register): Use new helper
> sframe_fre_get_cfa_offset.
>
> libsframe/
> * sframe.c (sframe_fre_get_cfa_offset): For S390 use
> SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE on CFA offset to undo its
> adjustment.
> * doc/sframe-spec.texi (S390,
> SFRAME_S390_CFA_OFFSET_ADJUSTMENT,
> SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE,
> SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE): Document S390-specific
> adjustment of CFA offset.
>
> Suggested-by: Indu Bhagat <indu.bhagat@oracle.com>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>
> Notes (jremus):
> A test build of Glibc tag 2.41 on s390x libc.so shows a ~8.8% reduction in
> .sframe section size due to storing the SFrame CFA offsets adjusted by
> -160.
>
> Statistics for libc.so - base (no CFA offset adjustment):
>
> .sframe size: 169,749 bytes
>
> VALUE TOTAL MIN MAX AVG
> FDEs: 3652 - - -
> FREs/FDE: 15236 1 20 4
> Offsets/FDE: 29792 1 38 8
> 8-bit: 0 0 0 0
> 16-bit: 29792 1 38 8
> 32-bit: 0 0 0 0
> Offsets/FRE: 29792 1 3 1
> 8-bit: - 0 0 0
> 16-bit: - 1 3 1
> 32-bit: - 0 0 0
> O_Padd/FDE: 342 - - 0
> 8-bit: 0
> 16-bit: 342
> 32-bit: 0
>
> Statistics for libc.so - CFA offset adjustment:
>
> .sframe size: 154,757 bytes
>
> VALUE TOTAL MIN MAX AVG
> FDEs: 3654 - - -
> FREs/FDE: 15238 1 20 4
> Offsets/FDE: 29794 1 38 8
> 8-bit: 14992 1 38 4
> 16-bit: 14802 0 0 4
> 32-bit: 0 0 0 0
> Offsets/FRE: 29794 2 6 1
> 8-bit: - 1 3 0
> 16-bit: - 1 3 0
> 32-bit: - 0 0 0
> O_Padd/FDE: 342 - - 0
> 8-bit: 283
> 16-bit: 59
> 32-bit: 0
>
> bfd/elf64-s390.c | 4 ++--
> gas/gen-sframe.c | 18 +++++++++++++++++-
> include/sframe.h | 8 ++++++++
> libsframe/doc/sframe-spec.texi | 10 +++++++++-
> libsframe/sframe.c | 10 ++++++++--
> 5 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
> index 478d5b86ebb0..6183dfac8827 100644
> --- a/bfd/elf64-s390.c
> +++ b/bfd/elf64-s390.c
> @@ -621,8 +621,8 @@ struct elf_s390x_sframe_plt
> static const sframe_frame_row_entry elf_s390x_sframe_plt_fre =
> {
> 0, /* SFrame FRE start address. */
> - {0, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes. */
> - SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_2B) /* FRE info. */
> + { SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE (160), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* Offset bytes. */
> + SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info. */
> };
>
> /* SFrame helper object for PLT. */
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index a1823ec15738..8f53bb4c21ff 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -146,10 +146,26 @@ sframe_fre_set_cfa_base_reg (struct sframe_row_entry *fre,
> fre->merge_candidate = false;
> }
>
> +static offsetT
> +sframe_fre_get_cfa_offset (const struct sframe_row_entry * fre)
> +{
> + offsetT offset = fre->cfa_offset;
> +
> + /* For S390 undo adjustment of CFA offset (to enable 8-bit offsets). */
> + if (sframe_get_abi_arch () == SFRAME_ABI_S390_ENDIAN_BIG)
> + offset = SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE (offset);
> +
> + return offset;
> +}
> +
> static void
> sframe_fre_set_cfa_offset (struct sframe_row_entry *fre,
> offsetT cfa_offset)
> {
> + /* For S390 adjust CFA offset to enable 8-bit offsets. */
> + if (sframe_get_abi_arch () == SFRAME_ABI_S390_ENDIAN_BIG)
> + cfa_offset = SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE (cfa_offset);
> +
> fre->cfa_offset = cfa_offset;
> fre->merge_candidate = false;
> }
> @@ -1056,7 +1072,7 @@ sframe_xlate_do_def_cfa_register (struct sframe_xlate_ctx *xlate_ctx,
> return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented. */
> }
> sframe_fre_set_cfa_base_reg (cur_fre, cfi_insn->u.r);
> - sframe_fre_set_cfa_offset (cur_fre, last_fre->cfa_offset);
> + sframe_fre_set_cfa_offset (cur_fre, sframe_fre_get_cfa_offset (last_fre));
> cur_fre->merge_candidate = false;
>
> return SFRAME_XLATE_OK;
> diff --git a/include/sframe.h b/include/sframe.h
> index fb0b6100fa9e..325bc6b8b5dc 100644
> --- a/include/sframe.h
> +++ b/include/sframe.h
> @@ -364,6 +364,14 @@ typedef struct sframe_frame_row_entry_addr4
> #define SFRAME_FRE_TYPE_ADDR4_LIMIT \
> (1ULL << ((SFRAME_FRE_TYPE_ADDR4 * 2) * 8))
>
> +/* On S390, the CFA offset from CFA base register is by definition a minimum
> + of 160. Store it adjusted by -160 to enable use of 8-bit SFrame offsets. */
> +#define SFRAME_S390_CFA_OFFSET_ADJUSTMENT SFRAME_S390_SP_VAL_OFFSET
> +#define SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE(offset) \
> + ((offset) + SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
> +#define SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset) \
> + ((offset) - SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
> +
> /* On S390, the CFA is defined as SP at call site + 160. Therefore the
> SP value offset from CFA is -160. */
I am split on whether to recommend to succinctly weave in why the value
of "160" (because this is the size of the register save area per stack
frame). Not a biggie though, your statement is clear enough.
/* For the s390x ABI, the CFA is defined as: SP + STACKSIZE, where:
SP is the value of stack pointer at the call site in the previous
frame, and,
STACKSIZE is 160 (size of the register save area).
Therefore the SP value offset from CFA is -160. */
> #define SFRAME_S390_SP_VAL_OFFSET -160
Nit: Put -160 as "(-160)".
> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
> index bf5c94722d0a..f9a117d55cc8 100644
> --- a/libsframe/doc/sframe-spec.texi
> +++ b/libsframe/doc/sframe-spec.texi
> @@ -837,7 +837,15 @@ Hence, in summary:
> @section S390
>
> Irrespective of the ABI, the first stack offset is always used to locate the
> -CFA, by interpreting it as: CFA = @code{BASE_REG} + offset1.
> +CFA. On S390 the value of the offset is stored adjusted by the S390-specific
> +@code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT} to enable the use of signed 8-bit
> +offsets on S390.
> +S390-specific helpers @code{SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE} and
> +@code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE} are provided to perform and undo the
> +adjustment. The CFA offset can therefore be interpreted as:
> +CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT}
> +or
> +CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset1)}.
I think swapping the order is more readable ?
CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset1)}
or
CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT}
> The identification of the @code{BASE_REG} is done by using the
> @code{fre_cfa_base_reg_id} field in the SFrame FRE info word.
>
> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> index b4b3c0c41aa2..d64ffe16e580 100644
> --- a/libsframe/sframe.c
> +++ b/libsframe/sframe.c
> @@ -682,10 +682,16 @@ sframe_fre_get_base_reg_id (sframe_frame_row_entry *fre, int *errp)
> /* Get the CFA offset from the FRE. If the offset is invalid, sets errp. */
>
> int32_t
> -sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx ATTRIBUTE_UNUSED,
> +sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx,
> sframe_frame_row_entry *fre, int *errp)
> {
> - return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
> + int32_t offset = sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
> +
> + /* For S390 undo adjustment of CFA offset (to enable 8-bit offsets). */
> + if (sframe_decoder_get_abi_arch (dctx) == SFRAME_ABI_S390_ENDIAN_BIG)
> + offset = SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE (offset);
> +
> + return offset;
> }
>
> /* Get the FP offset from the FRE. If the offset is invalid, sets errp. */
Hello Indu,
thank you for the review feedback!
On 17.04.2025 20:53, Indu Bhagat wrote:
> On 4/2/25 9:12 AM, Jens Remus wrote:
>> In SFrame V2 the size of the one to three offsets following a SFrame FDE
>
> "following an SFrame FRE"
>
> Also, I would avoid saying "of the one to three offsets". The format
> by itself does not limit the number of offsets, though for all current
> practical usages, we see 1-3 offsets. It is, of course, not
> recommended to use the format to encode large number of offsets.
Sure, makes sense. Will amend accordingly.
> The reason to avoid saying so is that it can be confusing to see that
> in git logs.
>
>> can be either signed 8-bit, 16-bit, or 32-bit integer, which the largest
>> offset 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 [1] defines the CFA as stack pointer (SP) at call
>> site +160. [2] Therefore the minimum CFA offset from CFA base register
>> on s390x is 160. This does not fit into a signed 8-bit integer and
>> therefore effectively prevents any use of signed 8-bit SFrame offsets
>> on s390x.
>>
>> For s390x store the CFA offset from CFA base register adjusted by -160
>> to enable the use of signed 8-bit SFrame offsets.
>>
>> [1]: s390x ELF ABI, https://github.com/IBM/s390x-abi/releases
>> [2]: s390x ELF ABI, commit 4e38ad9c8a88 ("Document the CFA"),
>> https://github.com/IBM/s390x-abi/commit/4e38ad9c8a88
>> diff --git a/include/sframe.h b/include/sframe.h
>> index fb0b6100fa9e..325bc6b8b5dc 100644
>> --- a/include/sframe.h
>> +++ b/include/sframe.h
>> @@ -364,6 +364,14 @@ typedef struct sframe_frame_row_entry_addr4
>> #define SFRAME_FRE_TYPE_ADDR4_LIMIT \
>> (1ULL << ((SFRAME_FRE_TYPE_ADDR4 * 2) * 8))
>> +/* On S390, the CFA offset from CFA base register is by definition a minimum
>> + of 160. Store it adjusted by -160 to enable use of 8-bit SFrame offsets. */
>> +#define SFRAME_S390_CFA_OFFSET_ADJUSTMENT SFRAME_S390_SP_VAL_OFFSET
>> +#define SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE(offset) \
>> + ((offset) + SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
>> +#define SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset) \
>> + ((offset) - SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
>> +
>> /* On S390, the CFA is defined as SP at call site + 160. Therefore the
>> SP value offset from CFA is -160. */
>
> I am split on whether to recommend to succinctly weave in why the
> value of "160" (because this is the size of the register save area
> per stack frame). Not a biggie though, your statement is clear
> enough.
>
> /* For the s390x ABI, the CFA is defined as: SP + STACKSIZE, where:
> SP is the value of stack pointer at the call site in the previous frame, and,
> STACKSIZE is 160 (size of the register save area).
> Therefore the SP value offset from CFA is -160. */
I'll amend this.
>
>> #define SFRAME_S390_SP_VAL_OFFSET -160
>
> Nit: Put -160 as "(-160)".
Done.
>
>> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
>> index bf5c94722d0a..f9a117d55cc8 100644
>> --- a/libsframe/doc/sframe-spec.texi
>> +++ b/libsframe/doc/sframe-spec.texi
>> @@ -837,7 +837,15 @@ Hence, in summary:
>> @section S390
>> Irrespective of the ABI, the first stack offset is always used to locate the
>> -CFA, by interpreting it as: CFA = @code{BASE_REG} + offset1.
>> +CFA. On S390 the value of the offset is stored adjusted by the S390-specific
>> +@code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT} to enable the use of signed 8-bit
>> +offsets on S390.
>> +S390-specific helpers @code{SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE} and
>> +@code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE} are provided to perform and undo the
>> +adjustment. The CFA offset can therefore be interpreted as:
>> +CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT}
>> +or
>> +CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset1)}.
>
> I think swapping the order is more readable ?
>
> CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset1)}
> or
> CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT}
Done.
>> The identification of the @code{BASE_REG} is done by using the
>> @code{fre_cfa_base_reg_id} field in the SFrame FRE info word.
Regards,
Jens
On 22.04.2025 16:14, Jens Remus wrote:
> On 17.04.2025 20:53, Indu Bhagat wrote:
>> On 4/2/25 9:12 AM, Jens Remus wrote:
>>> /* On S390, the CFA is defined as SP at call site + 160. Therefore the
>>> SP value offset from CFA is -160. */
>>
>> I am split on whether to recommend to succinctly weave in why the
>> value of "160" (because this is the size of the register save area
>> per stack frame). Not a biggie though, your statement is clear
>> enough.
>>
>> /* For the s390x ABI, the CFA is defined as: SP + STACKSIZE, where:
>> SP is the value of stack pointer at the call site in the previous frame, and,
>> STACKSIZE is 160 (size of the register save area).
>> Therefore the SP value offset from CFA is -160. */
>
> I'll amend this.
After discussion with our s390x ELF ABI maintainer I have decided not to
amend this. While the ABI source uses \STACKSIZE this will get replaced
by the value 160 when formatting the ABI document. While the reasoning
is most likely correct, the ABI does not explain the offset (value). So
I would rather not start to do in SFrame code.
Regards,
Jens
@@ -621,8 +621,8 @@ struct elf_s390x_sframe_plt
static const sframe_frame_row_entry elf_s390x_sframe_plt_fre =
{
0, /* SFrame FRE start address. */
- {0, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes. */
- SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_2B) /* FRE info. */
+ { SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE (160), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* Offset bytes. */
+ SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info. */
};
/* SFrame helper object for PLT. */
@@ -146,10 +146,26 @@ sframe_fre_set_cfa_base_reg (struct sframe_row_entry *fre,
fre->merge_candidate = false;
}
+static offsetT
+sframe_fre_get_cfa_offset (const struct sframe_row_entry * fre)
+{
+ offsetT offset = fre->cfa_offset;
+
+ /* For S390 undo adjustment of CFA offset (to enable 8-bit offsets). */
+ if (sframe_get_abi_arch () == SFRAME_ABI_S390_ENDIAN_BIG)
+ offset = SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE (offset);
+
+ return offset;
+}
+
static void
sframe_fre_set_cfa_offset (struct sframe_row_entry *fre,
offsetT cfa_offset)
{
+ /* For S390 adjust CFA offset to enable 8-bit offsets. */
+ if (sframe_get_abi_arch () == SFRAME_ABI_S390_ENDIAN_BIG)
+ cfa_offset = SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE (cfa_offset);
+
fre->cfa_offset = cfa_offset;
fre->merge_candidate = false;
}
@@ -1056,7 +1072,7 @@ sframe_xlate_do_def_cfa_register (struct sframe_xlate_ctx *xlate_ctx,
return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented. */
}
sframe_fre_set_cfa_base_reg (cur_fre, cfi_insn->u.r);
- sframe_fre_set_cfa_offset (cur_fre, last_fre->cfa_offset);
+ sframe_fre_set_cfa_offset (cur_fre, sframe_fre_get_cfa_offset (last_fre));
cur_fre->merge_candidate = false;
return SFRAME_XLATE_OK;
@@ -364,6 +364,14 @@ typedef struct sframe_frame_row_entry_addr4
#define SFRAME_FRE_TYPE_ADDR4_LIMIT \
(1ULL << ((SFRAME_FRE_TYPE_ADDR4 * 2) * 8))
+/* On S390, the CFA offset from CFA base register is by definition a minimum
+ of 160. Store it adjusted by -160 to enable use of 8-bit SFrame offsets. */
+#define SFRAME_S390_CFA_OFFSET_ADJUSTMENT SFRAME_S390_SP_VAL_OFFSET
+#define SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE(offset) \
+ ((offset) + SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
+#define SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset) \
+ ((offset) - SFRAME_S390_CFA_OFFSET_ADJUSTMENT)
+
/* On S390, the CFA is defined as SP at call site + 160. Therefore the
SP value offset from CFA is -160. */
#define SFRAME_S390_SP_VAL_OFFSET -160
@@ -837,7 +837,15 @@ Hence, in summary:
@section S390
Irrespective of the ABI, the first stack offset is always used to locate the
-CFA, by interpreting it as: CFA = @code{BASE_REG} + offset1.
+CFA. On S390 the value of the offset is stored adjusted by the S390-specific
+@code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT} to enable the use of signed 8-bit
+offsets on S390.
+S390-specific helpers @code{SFRAME_V2_FRE_S390_CFA_OFFSET_ENCODE} and
+@code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE} are provided to perform and undo the
+adjustment. The CFA offset can therefore be interpreted as:
+CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390_CFA_OFFSET_ADJUSTMENT}
+or
+CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE(offset1)}.
The identification of the @code{BASE_REG} is done by using the
@code{fre_cfa_base_reg_id} field in the SFrame FRE info word.
@@ -682,10 +682,16 @@ sframe_fre_get_base_reg_id (sframe_frame_row_entry *fre, int *errp)
/* Get the CFA offset from the FRE. If the offset is invalid, sets errp. */
int32_t
-sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx ATTRIBUTE_UNUSED,
+sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx,
sframe_frame_row_entry *fre, int *errp)
{
- return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
+ int32_t offset = sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
+
+ /* For S390 undo adjustment of CFA offset (to enable 8-bit offsets). */
+ if (sframe_decoder_get_abi_arch (dctx) == SFRAME_ABI_S390_ENDIAN_BIG)
+ offset = SFRAME_V2_FRE_S390_CFA_OFFSET_DECODE (offset);
+
+ return offset;
}
/* Get the FP offset from the FRE. If the offset is invalid, sets errp. */