On Fri, Jun 14, 2019 at 8:31 AM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> "H.J. Lu" <hjl.to...@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.

>     {
>       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?

Thanks.

-- 
H.J.

Reply via email to