[PATCH 7/8] s390: Store SFrame CFA offset adjusted
Indu Bhagat
indu.bhagat@oracle.com
Thu Apr 17 18:53:10 GMT 2025
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. */
More information about the Binutils
mailing list