public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jens Remus <jremus@linux.ibm.com>
To: binutils@sourceware.org, Indu Bhagat <indu.bhagat@oracle.com>
Cc: Jens Remus <jremus@linux.ibm.com>,
	Andreas Krebbel <krebbel@linux.ibm.com>
Subject: [RFC PATCH 1/1] sframe: Represent FP without RA on stack
Date: Mon, 22 Apr 2024 17:58:57 +0200	[thread overview]
Message-ID: <20240422155857.2497684-2-jremus@linux.ibm.com> (raw)
In-Reply-To: <20240422155857.2497684-1-jremus@linux.ibm.com>

If an architecture uses both SFrame RA and FP tracking SFrame assumes
that the RA offset is the 2nd offset and the FP offset is the 3rd offset
following the SFrame FRE. An architecture does not need to store both on
the stack. SFrame cannot represent a FP without RA on stack, since it
cannot distinguish whether the 2nd offset is the RA or FP offset.

Use an invalid SFrame FRE RA offset value of zero as dummy padding to
represent the FP being saved on the stack when the RA is not saved on
the stack.

include/
	* sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining
	the invalid RA offset value used to represent a dummy padding
	offset.

gas/
	* gen-sframe.c (get_fre_num_offsets): Accommodate for dummy
	padding RA offset if FP without RA on stack.
	(sframe_get_fre_offset_size): Likewise.
	(output_sframe_row_entry): Write a dummy padding RA offset
	if FP without RA needs to be represented.

libsframe/
	* sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA
	offsets as if they were undefined. Display them as "u*" to
	distinguish them.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    This patch eliminates 497 occurrences of the warning "skipping SFrame
    FDE due to FP without RA on stack" for a build of glibc on s390x. For
    libc.so this increases the number of FDEs by 166 and the number of
    FREs by 861, while adding 337 dummy padding RA offsets. With a total
    of 28157 offsets the dummy padding offsets account for ~1.20 % of the
    offsets.
    
    SFrame statistics without patch:
    
        VALUE        TOTAL      MIN        MAX        AVG
        FDEs:        3478       -          -          -
        FREs/FDE:    14441      1          15         4
        Offsets/FDE: 28157      1          31         8
           8-bit:    0          0          0          0
          16-bit:    28157      1          31         8
          32-bit:    0          0          0          0
        Offsets/FRE: 28157      1          3          1
           8-bit:    -          0          0          0
          16-bit:    -          1          3          1
          32-bit:    -          0          0          0
    
    SFrame statistics with patch applied:
    
        VALUE        TOTAL      MIN        MAX        AVG
        FDEs:        3644       -          -          -
        FREs/FDE:    15302      1          20         4
        Offsets/FDE: 29944      1          38         8
           8-bit:    0          0          0          0
          16-bit:    29944      1          38         8
          32-bit:    0          0          0          0
        Offsets/FRE: 29944      1          3          1
           8-bit:    -          0          0          0
          16-bit:    -          1          3          1
          32-bit:    -          0          0          0
        O_Padd/FDE:  337        -          -          0
           8-bit:    0
          16-bit:    337
          32-bit:    0
    
    Note that on s390x the offsets are at minimum 16-bits in size, due to
    the mandatory CFA offset being at least 160.

 gas/gen-sframe.c        | 50 +++++++++++++++++++----------------------
 include/sframe.h        |  9 ++++++--
 libsframe/sframe-dump.c |  4 ++++
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 4cc86eb6c815..990b08d87953 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -347,7 +347,9 @@ get_fre_num_offsets (struct sframe_row_entry *sframe_fre)
     fre_num_offsets++;
 #ifdef SFRAME_FRE_RA_TRACKING
   if (sframe_ra_tracking_p ()
-      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+      && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK
+	  /* Accommodate for padding RA offset if FP without RA on stack.  */
+	  || sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK))
     fre_num_offsets++;
 #endif
   return fre_num_offsets;
@@ -371,9 +373,14 @@ sframe_get_fre_offset_size (struct sframe_row_entry *sframe_fre)
   if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
     bp_offset_size = get_offset_size_in_bytes (sframe_fre->bp_offset);
 #ifdef SFRAME_FRE_RA_TRACKING
-  if (sframe_ra_tracking_p ()
-      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
-    ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
+  if (sframe_ra_tracking_p ())
+    {
+      if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+	ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
+      /* Accommodate for padding RA offset if FP without RA on stack.  */
+      else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
+	ra_offset_size = get_offset_size_in_bytes (SFRAME_FRE_RA_OFFSET_INVALID);
+    }
 #endif
 
   /* Get the maximum size needed to represent the offsets.  */
@@ -537,11 +544,19 @@ output_sframe_row_entry (symbolS *fde_start_addr,
   fre_write_offsets++;
 
 #ifdef SFRAME_FRE_RA_TRACKING
-  if (sframe_ra_tracking_p ()
-      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+  if (sframe_ra_tracking_p ())
     {
-      fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
-      fre_write_offsets++;
+      if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+	{
+	  fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
+	  fre_write_offsets++;
+	}
+      /* Write padding RA offset if FP without RA on stack.  */
+      else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
+	{
+	  fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
+	  fre_write_offsets++;
+	}
     }
 #endif
   if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
@@ -1497,25 +1512,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
 	= get_dw_fde_end_addrS (xlate_ctx->dw_fde);
     }
 
-#ifdef SFRAME_FRE_RA_TRACKING
-  if (sframe_ra_tracking_p ())
-    {
-      struct sframe_row_entry *fre;
-
-      /* Iterate over the scratchpad FREs and validate them.  */
-      for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
-	{
-	  /* SFrame format cannot represent FP on stack without RA on stack.  */
-	  if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
-	      && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
-	    {
-	      as_warn (_("skipping SFrame FDE due to FP without RA on stack"));
-	      return SFRAME_XLATE_ERR_NOTREPRESENTED;
-	    }
-	}
-    }
-#endif /* SFRAME_FRE_RA_TRACKING  */
-
   return SFRAME_XLATE_OK;
 }
 
diff --git a/include/sframe.h b/include/sframe.h
index 90bc92a32f84..d1a26875b3e2 100644
--- a/include/sframe.h
+++ b/include/sframe.h
@@ -237,6 +237,9 @@ typedef struct sframe_func_desc_entry
    may or may not be tracked.  */
 #define SFRAME_FRE_FP_OFFSET_IDX    2
 
+/* Invalid RA offset.  Used as padding to represent FP without RA on stack.  */
+#define SFRAME_FRE_RA_OFFSET_INVALID 0
+
 typedef struct sframe_fre_info
 {
   /* Information about
@@ -288,9 +291,11 @@ typedef struct sframe_fre_info
     offset1 (interpreted as CFA = BASE_REG + offset1)
 
     if RA is being tracked
-      offset2 (interpreted as RA = CFA + offset2)
+      offset2 (interpreted as RA = CFA + offset2; an offset value of
+	       SFRAME_FRE_RA_OFFSET_INVALID indicates a dummy padding RA offset
+	       to represent FP without RA saved on stack)
       if FP is being tracked
-	offset3 (intrepreted as FP = CFA + offset2)
+	offset3 (intrepreted as FP = CFA + offset3)
       fi
     else
       if FP is being tracked
diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
index 40ea531314ba..3ea4bc327efd 100644
--- a/libsframe/sframe-dump.c
+++ b/libsframe/sframe-dump.c
@@ -199,6 +199,10 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
       if (sframe_decoder_get_fixed_ra_offset (sfd_ctx)
 	  != SFRAME_CFA_FIXED_RA_INVALID)
 	strcpy (temp, "f");
+      /* If an ABI does track RA offset, e.g. AArch64 and S390, it can be a
+	 dummy as padding to represent FP without RA being saved on stack.  */
+      else if (err[2] == 0 && ra_offset == SFRAME_FRE_RA_OFFSET_INVALID)
+	sprintf (temp, "u*");
       else if (err[2] == 0)
 	{
 	  if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1))
-- 
2.40.1


  reply	other threads:[~2024-04-22 15:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-22 15:58 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding) Jens Remus
2024-04-22 15:58 ` Jens Remus [this message]
2024-04-22 23:58   ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Indu Bhagat
2024-04-23 15:44     ` Jens Remus
2024-04-25  6:59       ` Indu Bhagat
2024-04-22 15:59 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap) Jens Remus
2024-04-22 15:59 ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Jens Remus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240422155857.2497684-2-jremus@linux.ibm.com \
    --to=jremus@linux.ibm.com \
    --cc=binutils@sourceware.org \
    --cc=indu.bhagat@oracle.com \
    --cc=krebbel@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).