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. acsaw...@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC ToolchainIndex: 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 ();
}
-/* 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);
