public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Indu Bhagat <indu.bhagat@oracle.com>
To: binutils@sourceware.org
Cc: weimin.pan@oracle.com, Indu Bhagat <indu.bhagat@oracle.com>
Subject: [PATCH 4/6] gas: sframe: fine tune the fragment fixup for SFrame func info
Date: Wed,  7 Dec 2022 11:52:20 -0800	[thread overview]
Message-ID: <20221207195222.1182788-5-indu.bhagat@oracle.com> (raw)
In-Reply-To: <20221207195222.1182788-1-indu.bhagat@oracle.com>

SFrame function info is an unsigned 8-bit field comprising of the following
(from LSB to MSB):
  - 4-bits: FRE type
  - 1-bit: FRE start address encoding
  - 3-bits: Unused

At the moment, the most-significat 4-bits are zero (The FRE start
address encoding of SFRAME_FDE_TYPE_PCINC has a value of zero, and the upper
3-bits are unused). So the current implementation works without this patch.

To be precise, however, the fragment fixup logic is meant to fixup only the
least-significant 4-bits (i.e., only the FRE type needs to be updated
according to the function size).

This patch makes the gas implementation a bit more resilient: In the
future, when the format does evolve to make use of the currently unused
3-bits in various ways, the values in those 3-bits can be propagated
unchanged while the fragment fixup continues to update the lowermost
4-bits to indicate the selected FRE type.

ChangeLog:

	* gas/gen-sframe.c (create_func_info_exp): New definition.
	(output_sframe_funcdesc): Call create_func_info_exp.
	* gas/sframe-opt.c (sframe_estimate_size_before_relax): The
	associated fragment uses O_modulus now.
	(sframe_convert_frag): Adjust the fragment fixup code according
	to the new composite exp.
---
 gas/gen-sframe.c | 55 ++++++++++++++++++++++++++++++++++++++----------
 gas/sframe-opt.c | 34 +++++++++++++++++++++---------
 2 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 896fbe44c6b..075720facd6 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -396,8 +396,8 @@ sframe_get_fre_offset_size (struct sframe_row_entry *sframe_fre)
     - <val> and <width> are themselves expressionS.
     - <val> stores the expression which when evaluated gives the value of the
       start address offset of the FRE.
-    - <width> stores the expression when when evaluated gives the number of
-      bytes needed to encode the start address offset of the FRE.
+    - <width> stores the expression when evaluated gives the number of bytes
+      needed to encode the start address offset of the FRE.
 
    The use of OP_absent as the X_op_symbol helps identify this expression
    later when fragments are fixed up.  */
@@ -431,6 +431,41 @@ create_fre_start_addr_exp (expressionS *cexp, symbolS *fre_pc_begin,
   cexp->X_add_number = 0;
 }
 
+/* Create a composite exression CEXP (for SFrame FDE function info) such that:
+
+      exp = <rest_of_func_info> OP_modulus <width>, where,
+
+    - <rest_of_func_info> and <width> are themselves expressionS.
+    - <rest_of_func_info> stores a constant expression where X_add_number is
+    used to stash away the func_info.  The upper 4-bits of the func_info are copied
+    back to the resulting byte by the fragment fixup logic.
+    - <width> stores the expression when evaluated gives the size of the
+    funtion in number of bytes.
+
+   The use of OP_modulus as the X_op_symbol helps identify this expression
+   later when fragments are fixed up.  */
+
+static void
+create_func_info_exp (expressionS *cexp, symbolS *dw_fde_end_addrS,
+		      symbolS *dw_fde_start_addrS, uint8_t func_info)
+{
+  expressionS width;
+  expressionS rest_of_func_info;
+
+  width.X_op = O_subtract;
+  width.X_add_symbol = dw_fde_end_addrS;
+  width.X_op_symbol = dw_fde_start_addrS;
+  width.X_add_number = 0;
+
+  rest_of_func_info.X_op = O_constant;
+  rest_of_func_info.X_add_number = func_info;
+
+  cexp->X_op = O_modulus;
+  cexp->X_add_symbol = make_expr_symbol (&rest_of_func_info);
+  cexp->X_op_symbol = make_expr_symbol (&width);
+  cexp->X_add_number = 0;
+}
+
 #endif
 
 static void
@@ -538,19 +573,17 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
   out_four (sframe_fde->num_fres);
 
   /* SFrame FDE function info.  */
+  unsigned char func_info;
+  func_info = sframe_set_func_info (SFRAME_FDE_TYPE_PCINC,
+				    SFRAME_FRE_TYPE_ADDR4);
 #if SFRAME_FRE_TYPE_SELECTION_OPT
-  expressionS width;
-  width.X_op = O_subtract;
-  width.X_add_symbol = dw_fde_end_addrS;
-  width.X_op_symbol = dw_fde_start_addrS;
-  width.X_add_number = 0;
+  expressionS cexp;
+  create_func_info_exp (&cexp, dw_fde_end_addrS, dw_fde_start_addrS,
+			func_info);
   frag_grow (1); /* Size of func info is unsigned char.  */
   frag_var (rs_sframe, 1, 0, (relax_substateT) 0,
-	    make_expr_symbol (&width), 0, (char *) frag_now);
+	    make_expr_symbol (&cexp), 0, (char *) frag_now);
 #else
-  unsigned char func_info;
-  func_info = sframe_set_func_info (SFRAME_FDE_TYPE_PCINC,
-				    SFRAME_FRE_TYPE_ADDR4);
   out_one (func_info);
 #endif
 }
diff --git a/gas/sframe-opt.c b/gas/sframe-opt.c
index 6901aa82a77..f08a424fd88 100644
--- a/gas/sframe-opt.c
+++ b/gas/sframe-opt.c
@@ -40,10 +40,10 @@ sframe_estimate_size_before_relax (fragS *frag)
      The two kind of fragments can be differentiated based on the opcode
      of the symbol.  */
   exp = symbol_get_value_expression (frag->fr_symbol);
-  gas_assert ((exp->X_op == O_subtract) || (exp->X_op == O_absent));
+  gas_assert ((exp->X_op == O_modulus) || (exp->X_op == O_absent));
   /* Fragment for function info in an SFrame FDE will always write
      only one byte.  */
-  if (exp->X_op == O_subtract)
+  if (exp->X_op == O_modulus)
     ret = 1;
   /* Fragment for the start address in an SFrame FRE may write out
      1/2/4 bytes depending on the value of the diff.  */
@@ -92,8 +92,12 @@ sframe_convert_frag (fragS *frag)
   offsetT fsize;
   offsetT diff;
   offsetT value;
-  unsigned char func_info = SFRAME_FRE_TYPE_ADDR4;
+
+  offsetT rest_of_data;
+  uint8_t fde_type, fre_type;
+
   expressionS *exp;
+  symbolS *dataS;
   symbolS *fsizeS, *diffS;
 
   /* We are dealing with two different kind of fragments here which need
@@ -103,19 +107,29 @@ sframe_convert_frag (fragS *frag)
      The two kind of fragments can be differentiated based on the opcode
      of the symbol.  */
   exp = symbol_get_value_expression (frag->fr_symbol);
-  gas_assert ((exp->X_op == O_subtract) || (exp->X_op == O_absent));
+  gas_assert ((exp->X_op == O_modulus) || (exp->X_op == O_absent));
   /* Fragment for function info in an SFrame FDE.  */
-  if (exp->X_op == O_subtract)
+  if (exp->X_op == O_modulus)
     {
-      fsizeS = frag->fr_symbol;
+      /* Gather the existing value of the rest of the data except
+	 the fre_type.  */
+      dataS = exp->X_add_symbol;
+      rest_of_data = (symbol_get_value_expression(dataS))->X_add_number;
+      fde_type = SFRAME_V1_FUNC_FDE_TYPE (rest_of_data);
+      gas_assert (fde_type == SFRAME_FDE_TYPE_PCINC);
+
+      /* Calculate the applicable fre_type.  */
+      fsizeS = exp->X_op_symbol;
       fsize = resolve_symbol_value (fsizeS);
       if (fsize < SFRAME_FRE_TYPE_ADDR1_LIMIT)
-	func_info = SFRAME_FRE_TYPE_ADDR1;
+	fre_type = SFRAME_FRE_TYPE_ADDR1;
       else if (fsize < SFRAME_FRE_TYPE_ADDR2_LIMIT)
-	func_info = SFRAME_FRE_TYPE_ADDR2;
+	fre_type = SFRAME_FRE_TYPE_ADDR2;
       else
-	func_info = SFRAME_FRE_TYPE_ADDR4;
-      value = func_info;
+	fre_type = SFRAME_FRE_TYPE_ADDR4;
+
+      /* Create the new function info.  */
+      value = SFRAME_V1_FUNC_INFO (fde_type, fre_type);
 
       frag->fr_literal[frag->fr_fix] = value;
     }
-- 
2.37.2


  parent reply	other threads:[~2022-12-07 19:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-07 19:52 [PATCH 0/6] Small improvements around SFrame support Indu Bhagat
2022-12-07 19:52 ` [PATCH 1/6] libsframe: minor formatting nits Indu Bhagat
2022-12-08 11:03   ` Nick Clifton
2022-12-07 19:52 ` [PATCH 2/6] sframe.h: make some macros more precise Indu Bhagat
2022-12-07 23:52   ` Hans-Peter Nilsson
2022-12-08 17:36     ` Indu Bhagat
2022-12-08 20:26     ` [PATCH,V2 " Indu Bhagat
2022-12-12 16:02       ` Nick Clifton
2022-12-07 19:52 ` [PATCH 3/6] sframe: gas: libsframe: define constants and remove magic numbers Indu Bhagat
2022-12-08 11:10   ` Nick Clifton
2022-12-08 17:43     ` Indu Bhagat
2022-12-08 18:38       ` Indu Bhagat
2022-12-07 19:52 ` Indu Bhagat [this message]
2022-12-08 11:12   ` [PATCH 4/6] gas: sframe: fine tune the fragment fixup for SFrame func info Nick Clifton
2022-12-07 19:52 ` [PATCH 5/6] libsframe: rename API sframe_fde_func_info to sframe_fde_create_func_info Indu Bhagat
2022-12-08 11:13   ` Nick Clifton
2022-12-07 19:52 ` [PATCH 6/6] objdump: sframe: fix memory leaks Indu Bhagat
2022-12-08 11:14   ` Nick Clifton

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=20221207195222.1182788-5-indu.bhagat@oracle.com \
    --to=indu.bhagat@oracle.com \
    --cc=binutils@sourceware.org \
    --cc=weimin.pan@oracle.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).