On Tue, 2018-01-30 at 11:13 +0100, Jakub Jelinek wrote:
> On Tue, Jan 30, 2018 at 03:55:58AM -0600, Segher Boessenkool wrote:
> > > But in that case, what does the copying?
> >
> > I don't know. Aaron will look at it, but timezones etc. :-)
Indeed I did see unshare_all_rtl() copying internal_arg_pointer. But
also several places in function.c:
assign_parm_adjust_entry_rtl:
move_block_from_reg (REGNO (entry_parm),
validize_mem (copy_rtx (stack_parm)),
data->partial / UNITS_PER_WORD);
assign_parm_setup_reg:
/* Copy the value into the register, thus bridging between
assign_parm_find_data_types and expand_expr_real_1. */
equiv_stack_parm = data->stack_parm;
validated_mem = validize_mem (copy_rtx (data->entry_parm));
assign_parm_setup_block:
mem = validize_mem (copy_rtx (stack_parm));
in expr.c:
expand_expr_real_1:
/* DECL_MODE might change when TYPE_MODE depends on attribute target
settings for VECTOR_TYPE_P that might switch for the function. */
if (currently_expanding_to_rtl
&& code == VAR_DECL && MEM_P (decl_rtl)
&& VECTOR_TYPE_P (type) && exp && DECL_MODE (exp) != mode)
decl_rtl = change_address (decl_rtl, TYPE_MODE (type), 0);
else
decl_rtl = copy_rtx (decl_rtl);
> >
> > > That's what seems strange. I can see why we'd have two nested
> > > pluses with the inner plus being pointer-equal to
> > > internal_arg_ptr.
> > > And I can see why we'd have a single canonical plus (which IMO
> > > would
> > > be better, but I agree it's not stage 4 material). It's having
> > > the two
> > > nested pluses without maintaining pointer equality that seems
> > > strange.
> >
> > The inner plus is *not* pointer-equal, that is the
> > problem. Something
> > did copy_rtx (or such) on it, many things do. We can tell you what
> > exactly later today.
>
> Most likely unshare_all_rtl, which does:
> for (tree decl = DECL_ARGUMENTS (cfun->decl); decl; decl =
> DECL_CHAIN (decl))
> {
> if (DECL_RTL_SET_P (decl))
> SET_DECL_RTL (decl, copy_rtx_if_shared (DECL_RTL (decl)));
> DECL_INCOMING_RTL (decl) = copy_rtx_if_shared
> (DECL_INCOMING_RTL (decl));
> }
>
> Anyway, my preference would be to change that gen_rtx_PLUS into
> stack_parm = crtl->args.internal_arg_pointer;
> if (!CONST_INT_P (offset_rtx))
> stack_parm = gen_rtx_PLUS (Pmode, stack_parm, offset_rtx);
> else if (offset_rtx != const0_rtx)
> stack_parm = plus_constant (Pmode, stack_parm, INTVAL
> (offset_rtx));
> stack_parm = gen_rtx_MEM (data->promoted_mode, stack_parm);
> and deal specially with GET_CODE (crtl->args.internal_arg_pointer)
> in var-tracking.c.
> rs6000/powerpcspe with -fsplit-stack are the only cases where
> crtl->args.internal_arg_pointer is not a REG, so just running libgo
> testsuite on powerpc{,64,64le} should cover it all.
I'll give this a try today when I get to the office.
Thanks,
Aaron
>
> Jakub
>
--
Aaron Sawdey, Ph.D. acsaw...@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
