diff mbox

[rs6000] remove implicit static var outputs of toc_relative_expr_p

Message ID 1498581837.6219.12.camel@linux.vnet.ibm.com
State New
Headers show

Commit Message

Aaron Sawdey June 27, 2017, 4:43 p.m. UTC 
So, this is to set things up so I can in a future patch separate out
the code that deals with optimizing byte swapping for LE on P8 into a
separate file.

The function toc_relative_expr_p implicitly sets two static vars
(tocrel_base and tocrel_offset) that are declared in rs6000.c. The real
purpose of this is to communicate between
print_operand/print_operand_address and rs6000_output_addr_const_extra,
which is called through the asm_out hook vector by something in the
call tree under output_addr_const.

This patch changes toc_relative_expr_p to make tocrel_base and
tocrel_offset be explicit const_rtx * args. All of the calls other than
print_operand/print_operand_address are changed to have local const_rtx
vars that are passed in. The statics in rs6000.c are now called
tocrel_base_oac and tocrel_offset_oac to reflect their use to
communicate across output_addr_const, and that is now the only thing
they are used for.

Bootstrap and regtest passes in trunk 249639 (to avoid the bootstrap
fail), ok for trunk?


2017-06-27  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (toc_relative_expr_p): Make tocrel_base
	and tocrel_offset be pointer args rather than implicitly using
	static versions.
	(legitimate_constant_pool_address_p, rs6000_emit_move,
	const_load_sequence_p, adjust_vperm): Add local tocrel_base and
	tocrel_offset and use in toc_relative_expr_p call.
	(print_operand, print_operand_address): Use static tocrel_base_oac
	and tocrel_offset_oac.
	(rs6000_output_addr_const_extra): Use static tocrel_base_oac and
	tocrel_offset_oac.

Comments

Segher Boessenkool June 27, 2017, 11:35 p.m. UTC | #1 
Hi Aaron,

On Tue, Jun 27, 2017 at 11:43:57AM -0500, Aaron Sawdey wrote:
> The function toc_relative_expr_p implicitly sets two static vars
> (tocrel_base and tocrel_offset) that are declared in rs6000.c. The real
> purpose of this is to communicate between
> print_operand/print_operand_address and rs6000_output_addr_const_extra,
> which is called through the asm_out hook vector by something in the
> call tree under output_addr_const.
> 
> This patch changes toc_relative_expr_p to make tocrel_base and
> tocrel_offset be explicit const_rtx * args. All of the calls other than
> print_operand/print_operand_address are changed to have local const_rtx
> vars that are passed in.

If those locals aren't used, can you arrange to call toc_relative_expr_p
with NULL instead?  Or are they always used?

> The statics in rs6000.c are now called
> tocrel_base_oac and tocrel_offset_oac to reflect their use to
> communicate across output_addr_const, and that is now the only thing
> they are used for.

Can't say I like those names, very cryptical.  Not that I know something
better, the short names as they were weren't very nice either.

> --- gcc/config/rs6000/rs6000.c	(revision 249639)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -8628,18 +8628,25 @@
>  	  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (base), Pmode));
>  }
>  
> -static const_rtx tocrel_base, tocrel_offset;
> +/* These are only used to pass through from print_operand/print_operand_address
> + * to rs6000_output_addr_const_extra over the intervening function 
> + * output_addr_const which is not target code.  */

No leading * in a block comment please.  (And you have a trailing space).

> +static const_rtx tocrel_base_oac, tocrel_offset_oac;
>  
>  /* Return true if OP is a toc pointer relative address (the output
>     of create_TOC_reference).  If STRICT, do not match non-split
> -   -mcmodel=large/medium toc pointer relative addresses.  */
> +   -mcmodel=large/medium toc pointer relative addresses.  Places base 
> +   and offset pieces in TOCREL_BASE and TOCREL_OFFSET respectively.  */

s/Places/Place/ (and another trailing space).

> -  tocrel_base = op;
> -  tocrel_offset = const0_rtx;
> +  *tocrel_base = op;
> +  *tocrel_offset = const0_rtx;
>    if (GET_CODE (op) == PLUS && add_cint_operand (XEXP (op, 1), GET_MODE (op)))
>      {
> -      tocrel_base = XEXP (op, 0);
> -      tocrel_offset = XEXP (op, 1);
> +      *tocrel_base = XEXP (op, 0);
> +      *tocrel_offset = XEXP (op, 1);
>      }

Maybe write this as

  if (GET_CODE (op) == PLUS && add_cint_operand (XEXP (op, 1), GET_MODE (op)))
    {
      *tocrel_base = XEXP (op, 0);
      *tocrel_offset = XEXP (op, 1);
    }
  else
    {
      *tocrel_base = op;
      *tocrel_offset = const0_rtx;
    }

or, if you allow NULL pointers,

  bool with_offset = GET_CODE (op) == PLUS
		     && add_cint_operand (XEXP (op, 1), GET_MODE (op));
  if (tocrel_base)
    *tocrel_base = with_offset ? XEXP (op, 0) : op;
  if (tocrel_offset)
    *tocrel_offset = with_offset ? XEXP (op, 1) : const0_rtx;

or such.

> -  return (GET_CODE (tocrel_base) == UNSPEC
> -	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
> +  return (GET_CODE (*tocrel_base) == UNSPEC
> +	  && XINT (*tocrel_base, 1) == UNSPEC_TOCREL);

Well, and then you have this, so you need to assign tocrel_base to a local
as well.

>  legitimate_constant_pool_address_p (const_rtx x, machine_mode mode,
>  				    bool strict)
>  {
> -  return (toc_relative_expr_p (x, strict)
> +  const_rtx tocrel_base, tocrel_offset;
> +  return (toc_relative_expr_p (x, strict, &tocrel_base, &tocrel_offset)

For example here it seems nothing uses tocrel_base?

It is probably nicer to have a separate function for toc_relative_expr_p
and one to pull the base/offset out.  And maybe don't keep it cached for
the output function either?  It has all info it needs, right, the full
address RTX?  I don't think it is measurably slower to pull the address
apart an extra time?


Segher
diff mbox

Patch

Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h	(revision 249639)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -40,7 +40,7 @@ 
 extern int small_data_operand (rtx, machine_mode);
 extern bool mem_operand_gpr (rtx, machine_mode);
 extern bool mem_operand_ds_form (rtx, machine_mode);
-extern bool toc_relative_expr_p (const_rtx, bool);
+extern bool toc_relative_expr_p (const_rtx, bool, const_rtx *, const_rtx *);
 extern void validate_condition_mode (enum rtx_code, machine_mode);
 extern bool legitimate_constant_pool_address_p (const_rtx, machine_mode,
 						bool);
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 249639)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -8628,18 +8628,25 @@ 
 	  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (base), Pmode));
 }
 
-static const_rtx tocrel_base, tocrel_offset;
+/* These are only used to pass through from print_operand/print_operand_address
+ * to rs6000_output_addr_const_extra over the intervening function 
+ * output_addr_const which is not target code.  */
+static const_rtx tocrel_base_oac, tocrel_offset_oac;
 
 /* Return true if OP is a toc pointer relative address (the output
    of create_TOC_reference).  If STRICT, do not match non-split
-   -mcmodel=large/medium toc pointer relative addresses.  */
+   -mcmodel=large/medium toc pointer relative addresses.  Places base 
+   and offset pieces in TOCREL_BASE and TOCREL_OFFSET respectively.  */
 
 bool
-toc_relative_expr_p (const_rtx op, bool strict)
+toc_relative_expr_p (const_rtx op, bool strict, const_rtx *tocrel_base,
+		     const_rtx *tocrel_offset)
 {
   if (!TARGET_TOC)
     return false;
 
+  gcc_assert (tocrel_base != NULL && tocrel_offset != NULL);
+
   if (TARGET_CMODEL != CMODEL_SMALL)
     {
       /* When strict ensure we have everything tidy.  */
@@ -8655,16 +8662,16 @@ 
 	op = XEXP (op, 1);
     }
 
-  tocrel_base = op;
-  tocrel_offset = const0_rtx;
+  *tocrel_base = op;
+  *tocrel_offset = const0_rtx;
   if (GET_CODE (op) == PLUS && add_cint_operand (XEXP (op, 1), GET_MODE (op)))
     {
-      tocrel_base = XEXP (op, 0);
-      tocrel_offset = XEXP (op, 1);
+      *tocrel_base = XEXP (op, 0);
+      *tocrel_offset = XEXP (op, 1);
     }
 
-  return (GET_CODE (tocrel_base) == UNSPEC
-	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
+  return (GET_CODE (*tocrel_base) == UNSPEC
+	  && XINT (*tocrel_base, 1) == UNSPEC_TOCREL);
 }
 
 /* Return true if X is a constant pool address, and also for cmodel=medium
@@ -8674,7 +8681,8 @@ 
 legitimate_constant_pool_address_p (const_rtx x, machine_mode mode,
 				    bool strict)
 {
-  return (toc_relative_expr_p (x, strict)
+  const_rtx tocrel_base, tocrel_offset;
+  return (toc_relative_expr_p (x, strict, &tocrel_base, &tocrel_offset)
 	  && (TARGET_CMODEL != CMODEL_MEDIUM
 	      || constant_pool_expr_p (XVECEXP (tocrel_base, 0, 0))
 	      || mode == QImode
@@ -11055,6 +11063,7 @@ 
       /* If this is a SYMBOL_REF that refers to a constant pool entry,
 	 and we have put it in the TOC, we just need to make a TOC-relative
 	 reference to it.  */
+      const_rtx tocrel_base, tocrel_offset;
       if (TARGET_TOC
 	  && GET_CODE (operands[1]) == SYMBOL_REF
 	  && use_toc_relative_ref (operands[1], mode))
@@ -11069,7 +11078,7 @@ 
 			   > (TARGET_CMODEL != CMODEL_SMALL ? 3 : 2)))
 		   || (GET_CODE (operands[0]) == REG
 		       && FP_REGNO_P (REGNO (operands[0]))))
-	       && !toc_relative_expr_p (operands[1], false)
+	       && !toc_relative_expr_p (operands[1], false, &tocrel_base, &tocrel_offset)
 	       && (TARGET_CMODEL == CMODEL_SMALL
 		   || can_create_pseudo_p ()
 		   || (REG_P (operands[0])
@@ -21812,7 +21821,7 @@ 
 	}
       else
 	{
-	  if (toc_relative_expr_p (x, false))
+	  if (toc_relative_expr_p (x, false, &tocrel_base_oac, &tocrel_offset_oac))
 	    /* This hack along with a corresponding hack in
 	       rs6000_output_addr_const_extra arranges to output addends
 	       where the assembler expects to find them.  eg.
@@ -21819,7 +21828,7 @@ 
 	       (plus (unspec [(symbol_ref ("x")) (reg 2)] tocrel) 4)
 	       without this hack would be output as "x@toc+4".  We
 	       want "x+4@toc".  */
-	    output_addr_const (file, CONST_CAST_RTX (tocrel_base));
+	    output_addr_const (file, CONST_CAST_RTX (tocrel_base_oac));
 	  else
 	    output_addr_const (file, x);
 	}
@@ -21886,7 +21895,7 @@ 
       fprintf (file, "@l(%s)", reg_names[ REGNO (XEXP (x, 0)) ]);
     }
 #endif
-  else if (toc_relative_expr_p (x, false))
+  else if (toc_relative_expr_p (x, false, &tocrel_base_oac, &tocrel_offset_oac))
     {
       /* This hack along with a corresponding hack in
 	 rs6000_output_addr_const_extra arranges to output addends
@@ -21895,17 +21904,17 @@ 
 	 .       (plus (unspec [(symbol_ref ("x")) (reg 2)] tocrel) 8))
 	 without this hack would be output as "x@toc+8@l(9)".  We
 	 want "x+8@toc@l(9)".  */
-      output_addr_const (file, CONST_CAST_RTX (tocrel_base));
+      output_addr_const (file, CONST_CAST_RTX (tocrel_base_oac));
       if (GET_CODE (x) == LO_SUM)
 	fprintf (file, "@l(%s)", reg_names[REGNO (XEXP (x, 0))]);
       else
-	fprintf (file, "(%s)", reg_names[REGNO (XVECEXP (tocrel_base, 0, 1))]);
+	fprintf (file, "(%s)", reg_names[REGNO (XVECEXP (tocrel_base_oac, 0, 1))]);
     }
   else
     gcc_unreachable ();
 }
 
-/* Implement TARGET_OUTPUT_ADDR_CONST_EXTRA.  */
+/* Implement TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA.  */
 
 static bool
 rs6000_output_addr_const_extra (FILE *file, rtx x)
@@ -21918,11 +21927,11 @@ 
 			     && REG_P (XVECEXP (x, 0, 1))
 			     && REGNO (XVECEXP (x, 0, 1)) == TOC_REGISTER);
 	output_addr_const (file, XVECEXP (x, 0, 0));
-	if (x == tocrel_base && tocrel_offset != const0_rtx)
+	if (x == tocrel_base_oac && tocrel_offset_oac != const0_rtx)
 	  {
-	    if (INTVAL (tocrel_offset) >= 0)
+	    if (INTVAL (tocrel_offset_oac) >= 0)
 	      fprintf (file, "+");
-	    output_addr_const (file, CONST_CAST_RTX (tocrel_offset));
+	    output_addr_const (file, CONST_CAST_RTX (tocrel_offset_oac));
 	  }
 	if (!TARGET_AIX || (TARGET_ELF && TARGET_MINIMAL_TOC))
 	  {
@@ -39312,6 +39321,8 @@ 
   if (!insn_entry[uid].is_swap || insn_entry[uid].is_load)
     return false;
 
+  const_rtx tocrel_base, tocrel_offset;
+
   /* Find the unique use in the swap and locate its def.  If the def
      isn't unique, punt.  */
   struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
@@ -39357,7 +39368,7 @@ 
 	  rtx tocrel_expr = SET_SRC (tocrel_body);
 	  if (GET_CODE (tocrel_expr) == MEM)
 	    tocrel_expr = XEXP (tocrel_expr, 0);
-	  if (!toc_relative_expr_p (tocrel_expr, false))
+	  if (!toc_relative_expr_p (tocrel_expr, false, &tocrel_base, &tocrel_offset))
 	    return false;
 	  split_const (XVECEXP (tocrel_base, 0, 0), &base, &offset);
 	  if (GET_CODE (base) != SYMBOL_REF || !CONSTANT_POOL_ADDRESS_P (base))
@@ -40118,11 +40129,12 @@ 
      to set tocrel_base; otherwise it would be unnecessary as we've
      already established it will return true.  */
   rtx base, offset;
+  const_rtx tocrel_base, tocrel_offset;
   rtx tocrel_expr = SET_SRC (PATTERN (tocrel_insn));
   /* There is an extra level of indirection for small/large code models.  */
   if (GET_CODE (tocrel_expr) == MEM)
     tocrel_expr = XEXP (tocrel_expr, 0);
-  if (!toc_relative_expr_p (tocrel_expr, false))
+  if (!toc_relative_expr_p (tocrel_expr, false, &tocrel_base, &tocrel_offset))
     gcc_unreachable ();
   split_const (XVECEXP (tocrel_base, 0, 0), &base, &offset);
   rtx const_vector = get_pool_constant (base);