V3: [PATCH] Update preferred_stack_boundary only when expanding function call

[Richard Sandiford]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Fri, Jun 14, 2019 at 8:31 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> > diff --git a/gcc/calls.c b/gcc/calls.c
>> > index c8a42680041..6ab138e7bb0 100644
>> > --- a/gcc/calls.c
>> > +++ b/gcc/calls.c
>> > @@ -3226,6 +3226,19 @@ can_implement_as_sibling_call_p (tree exp,
>> >    return true;
>> >  }
>> >
>> > +/* Update stack alignment when the parameter is passed in the stack
>> > +   since the outgoing parameter requires extra alignment on the calling
>> > +   function side. */
>> > +
>> > +static void
>> > +update_stack_alignment_for_call (struct locate_and_pad_arg_data *locate)
>> > +{
>> > +  if (crtl->stack_alignment_needed < locate->boundary)
>> > +    crtl->stack_alignment_needed = locate->boundary;
>> > +  if (crtl->preferred_stack_boundary < locate->boundary)
>> > +    crtl->preferred_stack_boundary = locate->boundary;
>> > +}
>> > +
>> >  /* Generate all the code for a CALL_EXPR exp
>> >     and return an rtx for its value.
>> >     Store the value in TARGET (specified as an rtx) if convenient.
>> > @@ -3703,6 +3716,12 @@ expand_call (tree exp, rtx target, int ignore)
>> >    /* Ensure current function's preferred stack boundary is at least
>> >       what we need.  Stack alignment may also increase preferred stack
>> >       boundary.  */
>> > +  for (i = 0; i < num_actuals; i++)
>> > +    if (reg_parm_stack_space > 0
>> > +     || args[i].reg == 0
>> > +     || args[i].partial != 0
>> > +     || args[i].pass_on_stack)
>> > +      update_stack_alignment_for_call (&args[i].locate);
>> >    if (crtl->preferred_stack_boundary < preferred_stack_boundary)
>> >      crtl->preferred_stack_boundary = preferred_stack_boundary;
>> >    else
>> > @@ -4961,6 +4980,12 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
>> >        targetm.calls.function_arg_advance (args_so_far, mode, (tree) 0, true);
>> >      }
>> >
>> > +  for (int i = 0; i < nargs; i++)
>> > +    if (reg_parm_stack_space > 0
>> > +     || argvec[i].reg == 0
>> > +     || argvec[i].partial != 0)
>> > +      update_stack_alignment_for_call (&argvec[i].locate);
>> > +
>>
>> It's safe to test argvec[i].pass_on_stack here too, since the vector
>
> There is no pass_on_stack in argvec:
>
>   struct arg
>   {
>     rtx value;
>     machine_mode mode;
>     rtx reg;
>     int partial;
>     struct locate_and_pad_arg_data locate;
>     rtx save_area;
>   };
>   struct arg *argvec;
>
>> is initialised to zeros.  So I think we should move the "if"s into the
>> new function:
>>
>> static void
>> update_stack_alignment_for_call (struct locate_and_pad_arg_data *locate)
>> {
>>   if (reg_parm_stack_space > 0
>>       || locate->reg == 0
>>       || locate->partial != 0
>>       || locate->pass_on_stack)
>
> Since we have
>
> struct locate_and_pad_arg_data
> {
>   /* Size of this argument on the stack, rounded up for any padding it
>      gets.  If REG_PARM_STACK_SPACE is defined, then register parms are
>      counted here, otherwise they aren't.  */
>   struct args_size size;
>   /* Offset of this argument from beginning of stack-args.  */
>   struct args_size offset;
>   /* Offset to the start of the stack slot.  Different from OFFSET
>      if this arg pads downward.  */
>   struct args_size slot_offset;
>   /* The amount that the stack pointer needs to be adjusted to
>      force alignment for the next argument.  */
>   struct args_size alignment_pad;
>   /* Which way we should pad this arg.  */
>   pad_direction where_pad;
>   /* slot_offset is at least this aligned.  */
>   unsigned int boundary;
> };
>
> we can't check reg, partial nor pass_on_stack here.

Sorry, missed that they were different structures.

>>     {
>>       if (crtl->stack_alignment_needed < locate->boundary)
>>         crtl->stack_alignment_needed = locate->boundary;
>>       if (crtl->preferred_stack_boundary < locate->boundary)
>>         crtl->preferred_stack_boundary = locate->boundary;
>>     }
>> }
>>
>> OK with that change, thanks.
>>
>
> Is my original patch OK?

Yes, thanks.

Richard