[PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[Aaron Sawdey]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

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.


-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

[-- Attachment #2: tocrel.patch --]
[-- Type: text/x-patch, Size: 7693 bytes --]

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 ();
 }
 \f
-/* 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);

Re: [PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[Segher Boessenkool]

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

Re: [PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[Aaron Sawdey]

[-- Attachment #1: Type: text/plain, Size: 6898 bytes --]

Hi Segher,

On Tue, 2017-06-27 at 18:35 -0500, Segher Boessenkool wrote:
> 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?

There are calls where neither is used or only tocrel_base is 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?

I think it doesn't make a lot of sense to have two functions as you
have to do nearly all the work just to get the true/false return value,
you have to completely compute tocrel_base. I've restructured this so
that either pointer can be null. The function uses locals for
tocrel_base/tocrel_offset, then assigns to the pointers if non-null.

bool
toc_relative_expr_p (const_rtx op, bool strict, const_rtx
*tocrel_base_ret,
		     const_rtx *tocrel_offset_ret)
{
  if (!TARGET_TOC)
    return false;

  if (TARGET_CMODEL != CMODEL_SMALL)
    {
      /* When strict ensure we have everything tidy.  */
      if (strict
	  && !(GET_CODE (op) == LO_SUM
	       && REG_P (XEXP (op, 0))
	       && INT_REG_OK_FOR_BASE_P (XEXP (op, 0), strict)))
	return false;

      /* When not strict, allow non-split TOC addresses and also allow
	 (lo_sum (high ..)) TOC addresses created during reload.  */
      if (GET_CODE (op) == LO_SUM)
	op = XEXP (op, 1);
    }

  const_rtx tocrel_base = op;
  const_rtx tocrel_offset = const0_rtx;

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

  if (tocrel_base_ret)
    *tocrel_base_ret = tocrel_base;
  if (tocrel_offset_ret)
    *tocrel_offset_ret = tocrel_offset;

  return (GET_CODE (tocrel_base) == UNSPEC
	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
}

Revised patch is attached, bootstrap/regtest in progress. If everything
passes, ok for trunk?

Thanks,
    Aaron

-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

[-- Attachment #2: tocrel.patch2 --]
[-- Type: text/x-patch, Size: 7077 bytes --]

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,14 +8628,19 @@
 	  && 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.  Place 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_ret,
+		     const_rtx *tocrel_offset_ret)
 {
   if (!TARGET_TOC)
     return false;
@@ -8655,14 +8660,21 @@
 	op = XEXP (op, 1);
     }
 
-  tocrel_base = op;
-  tocrel_offset = const0_rtx;
+  const_rtx tocrel_base = op;
+  const_rtx 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);
+      if (tocrel_offset_ret)
+	tocrel_offset = XEXP (op, 1);
     }
 
+  if (tocrel_base_ret)
+    *tocrel_base_ret = tocrel_base;
+  if (tocrel_offset_ret)
+    *tocrel_offset_ret = tocrel_offset;
+
   return (GET_CODE (tocrel_base) == UNSPEC
 	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
 }
@@ -8674,7 +8686,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
@@ -11069,7 +11082,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, NULL, NULL)
 	       && (TARGET_CMODEL == CMODEL_SMALL
 		   || can_create_pseudo_p ()
 		   || (REG_P (operands[0])
@@ -21812,7 +21825,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 +21832,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 +21899,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 +21908,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 ();
 }
 \f
-/* 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 +21931,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 +39325,8 @@
   if (!insn_entry[uid].is_swap || insn_entry[uid].is_load)
     return false;
 
+  const_rtx tocrel_base;
+
   /* 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 +39372,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, NULL))
 	    return false;
 	  split_const (XVECEXP (tocrel_base, 0, 0), &base, &offset);
 	  if (GET_CODE (base) != SYMBOL_REF || !CONSTANT_POOL_ADDRESS_P (base))
@@ -40118,11 +40133,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;
   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, NULL))
     gcc_unreachable ();
   split_const (XVECEXP (tocrel_base, 0, 0), &base, &offset);
   rtx const_vector = get_pool_constant (base);

Re: [PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[Segher Boessenkool]

On Wed, Jun 28, 2017 at 03:21:49PM -0500, Aaron Sawdey wrote:
> > 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?
> 
> I think it doesn't make a lot of sense to have two functions as you
> have to do nearly all the work just to get the true/false return value,
> you have to completely compute tocrel_base.

Right, but how much work is that?  And it will get rid of all the cached
stuff, simplifying things quite a bit.  None of this is new in your
patch of course.

>  /* 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.  Place base
> +   and offset pieces in TOCREL_BASE and TOCREL_OFFSET respectively.  */

"If those are non-NULL"?

> -toc_relative_expr_p (const_rtx op, bool strict)
> +toc_relative_expr_p (const_rtx op, bool strict, const_rtx *tocrel_base_ret,
> +		     const_rtx *tocrel_offset_ret)
>  {
>    if (!TARGET_TOC)
>      return false;

> -  tocrel_base = op;
> -  tocrel_offset = const0_rtx;
> +  const_rtx tocrel_base = op;
> +  const_rtx 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);
> +      if (tocrel_offset_ret)
> +	tocrel_offset = XEXP (op, 1);

Lose the "if"?  Or do you get a compiler warning then?

> @@ -8674,7 +8686,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

Use NULL for the args here, instead?

The patch is okay for trunk with those things taken care of.  Thanks,


Segher

Re: [PATCH rs6000] remove implicit static var outputs of toc_relative_expr_p

[Aaron Sawdey]

On Wed, 2017-06-28 at 18:19 -0500, Segher Boessenkool wrote:
> On Wed, Jun 28, 2017 at 03:21:49PM -0500, Aaron Sawdey wrote:
> > -toc_relative_expr_p (const_rtx op, bool strict)
> > +toc_relative_expr_p (const_rtx op, bool strict, const_rtx
> > *tocrel_base_ret,
> > +		     const_rtx *tocrel_offset_ret)
> >  {
> >    if (!TARGET_TOC)
> >      return false;
> > -  tocrel_base = op;
> > -  tocrel_offset = const0_rtx;
> > +  const_rtx tocrel_base = op;
> > +  const_rtx 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);
> > +      if (tocrel_offset_ret)
> > +	tocrel_offset = XEXP (op, 1);
> 
> Lose the "if"?  Or do you get a compiler warning then?

I was just trying to avoid unnecessary work in the case where the
pointer is NULL. In that case tocrel_offset isn't actually used for
anything. Probably I should just let the compiler figure that one out,
I will delete the if for clarity.

> > @@ -8674,7 +8686,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
> 
> Use NULL for the args here, instead?

The diff didn't include all the context. Both tocrel_base and
tocrel_offset are used in the function:

bool
legitimate_constant_pool_address_p (const_rtx x, machine_mode mode,
				    bool 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
	      || offsettable_ok_by_alignment (XVECEXP (tocrel_base, 0, 0),
					      INTVAL (tocrel_offset), mode)));
}


> 
> The patch is okay for trunk with those things taken care of.  Thanks,
> 
> 
> Segher
> 
-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain