diff mbox series

[4/6] gas: sframe: fine tune the fragment fixup for SFrame func info

Message ID 20221207195222.1182788-5-indu.bhagat@oracle.com
State Accepted
Headers
Series Small improvements around SFrame support |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Indu Bhagat Dec. 7, 2022, 7:52 p.m. UTC 
  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(-)

Comments

Nick Clifton Dec. 8, 2022, 11:12 a.m. UTC | #1 
Hi Indu,

> 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.

Approved - please apply.

Cheers
   Nick
diff mbox series

Patch

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;
     }