On Wed, Nov 20, 2024 at 2:12 AM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> "H.J. Lu" <hjl.to...@gmail.com> writes:
> > Adjust BLKmode argument size for parameter alignment for sibcall check.
> >
> > gcc/
> >
> > PR middle-end/117098
> > * calls.cc (store_one_arg): Adjust BLKmode argument size for
> > alignment padding for sibcall check.
> >
> > gcc/testsuite/
> >
> > PR middle-end/117098
> > * gcc.dg/sibcall-12.c: New test.
> >
> > OK for master?
> >
> >
> > H.J.
> > From 8b0518906cb23a9b5e77b04d6132c49047daebd2 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.to...@gmail.com>
> > Date: Sun, 13 Oct 2024 04:53:14 +0800
> > Subject: [PATCH] sibcall: Adjust BLKmode argument size for alignment padding
> >
> > Adjust BLKmode argument size for parameter alignment for sibcall check.
> >
> > gcc/
> >
> > PR middle-end/117098
> > * calls.cc (store_one_arg): Adjust BLKmode argument size for
> > alignment padding for sibcall check.
> >
> > gcc/testsuite/
> >
> > PR middle-end/117098
> > * gcc.dg/sibcall-12.c: New test.
> >
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> > gcc/calls.cc | 4 +++-
> > gcc/testsuite/gcc.dg/sibcall-12.c | 13 +++++++++++++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.dg/sibcall-12.c
> >
> > diff --git a/gcc/calls.cc b/gcc/calls.cc
> > index c5c26f65280..163c7e509d9 100644
> > --- a/gcc/calls.cc
> > +++ b/gcc/calls.cc
> > @@ -5236,7 +5236,9 @@ store_one_arg (struct arg_data *arg, rtx argblock,
> > int flags,
> > /* expand_call should ensure this. */
> > gcc_assert (!arg->locate.offset.var
> > && arg->locate.size.var == 0);
> > - poly_int64 size_val = rtx_to_poly_int64 (size_rtx);
> > + /* Adjust for argument alignment padding. */
> > + poly_int64 size_val = ROUND_UP (UINTVAL (size_rtx),
> > + parm_align / BITS_PER_UNIT);
>
> This doesn't look right to me. For one thing, going from
> rtx_to_poly_int64 to UINTVAL drops support for non-constant parameters.
> But even ignoring that, I think padding size_val (the size of arg->value
> IIUC) will pessimise the later:
>
> else if (maybe_in_range_p (arg->locate.offset.constant,
> i, size_val))
> sibcall_failure = true;
>
> and so cause sibcall failures elsewhere. I'm also not sure this
> accurately reproduces the padding that is added by locate_and_pad_parm
> for all cases (arguments that grow up vs down, padding below vs above
> the argument).
>
> AIUI, the point of the:
>
> if (known_eq (arg->locate.offset.constant, i))
> {
> /* Even though they appear to be at the same location,
> if part of the outgoing argument is in registers,
> they aren't really at the same location. Check for
> this by making sure that the incoming size is the
> same as the outgoing size. */
> if (maybe_ne (arg->locate.size.constant, size_val))
> sibcall_failure_1 = true;
> }
Does this
diff --git a/gcc/calls.cc b/gcc/calls.cc
index 246abe34243..98429cc757f 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -5327,7 +5327,13 @@ store_one_arg (struct arg_data *arg, rtx
argblock, int flags,
they aren't really at the same location. Check for
this by making sure that the incoming size is the
same as the outgoing size. */
- if (maybe_ne (arg->locate.size.constant, size_val))
+ poly_int64 aligned_size;
+ if (CONST_INT_P (size_rtx))
+ aligned_size = ROUND_UP (UINTVAL (size_rtx),
+ parm_align / BITS_PER_UNIT);
+ else
+ aligned_size = size_val;
+ if (maybe_ne (arg->locate.size.constant, aligned_size))
sibcall_failure = true;
}
else if (maybe_in_range_p (arg->locate.offset.constant,
look correct?
Thanks.
> that you cite in the PR is to make sure that the nth byte of arg->value
> corresponds to arg->locate.offset.constant + n. It's not clear to me
> why the original fix for PR32602 didn't just check partial != 0.
> Perhaps it was just for consistency with the neighbouring overlap check,
> or maybe it was for some deeper reason.
>
> Thanks,
> Richard
>
> >
> > if (known_eq (arg->locate.offset.constant, i))
> > {
> > diff --git a/gcc/testsuite/gcc.dg/sibcall-12.c
> > b/gcc/testsuite/gcc.dg/sibcall-12.c
> > new file mode 100644
> > index 00000000000..5773c9c1c4a
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/sibcall-12.c
> > @@ -0,0 +1,13 @@
> > +// Test for sibcall optimization with struct aligned on stack.
> > +// { dg-options "-O2" }
> > +// { dg-final { scan-assembler "jmp" { target i?86-*-* x86_64-*-* } } }
> > +
> > +struct A { char a[17]; };
> > +
> > +int baz (int a, int b, int c, void *p, struct A s, struct A);
> > +
> > +int
> > +foo (int a, int b, int c, void *p, struct A s, struct A s2)
> > +{
> > + return baz (a, b, c, p, s, s2);
> > +}
--
H.J.
