On Fri, Mar 19, 2021 at 01:04:19PM +0000, Richard Sandiford via Gcc-patches
wrote:
> > As the following testcase shows, we emit a -Wpsabi note about argument
> > passing change since GCC 9, but in reality the ABI didn't change.
> > The alignment is 8 bits in GCC < 9 and 32 bits in GCC >= 9 and
> > the aarch64_function_arg_alignment returns in that case:
> > return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> > so when both the old and new alignment are smaller or equal to PARM_BOUNDARY
> > (or both are larger than STACK_BOUNDARY, just in theory), even when the new
> > one is bigger, it doesn't change the argument passing.
> >
> > So, the following patch changes aarch64_function_arg_alignment to tell the
> > callers the exact old alignmentm so that they can test it if needed.
> > The other aarch64_function_arg_alignment callers either check the
> > alignment for equality against 16-byte alignment (when old alignment was
> > smaller than that and the new one is 16-byte, we want to emit -Wpsabi
> > in all the cases) or the va_arg case which I think is ok now too.
> >
> > Bootstrapped/regtested on aarch64-linux, ok for trunk?
>
> Looks good to me. Richard E knows this code better than I do though,
> so I think he should have the final say. He's currently on holiday
> but will be back next week.
I'd like to ping this patch.
Thanks.
> > 2021-03-18 Jakub Jelinek <ja...@redhat.com>
> >
> > PR target/91710
> > * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Change
> > abi_break argument from bool * to unsigned *, store there the pre-GCC 9
> > alignment.
> > (aarch64_layout_arg, aarch64_gimplify_va_arg_expr): Adjust callers.
> > (aarch64_function_arg_regno_p): Likewise. Only emit -Wpsabi note if
> > the old and new alignment after applying MIN/MAX to it is different.
> >
> > * gcc.target/aarch64/pr91710.c: New test.
> >
> > --- gcc/config/aarch64/aarch64.c.jj 2021-03-18 15:14:51.721425223 +0100
> > +++ gcc/config/aarch64/aarch64.c 2021-03-18 16:35:04.437115447 +0100
> > @@ -5938,9 +5938,9 @@ aarch64_vfp_is_call_candidate (cumulativ
> >
> > static unsigned int
> > aarch64_function_arg_alignment (machine_mode mode, const_tree type,
> > - bool *abi_break)
> > + unsigned int *abi_break)
> > {
> > - *abi_break = false;
> > + *abi_break = 0;
> > if (!type)
> > return GET_MODE_ALIGNMENT (mode);
> >
> > @@ -5982,7 +5982,7 @@ aarch64_function_arg_alignment (machine_
> >
> > if (bitfield_alignment > alignment)
> > {
> > - *abi_break = true;
> > + *abi_break = alignment;
> > return bitfield_alignment;
> > }
> >
> > @@ -6004,7 +6004,7 @@ aarch64_layout_arg (cumulative_args_t pc
> > int ncrn, nvrn, nregs;
> > bool allocate_ncrn, allocate_nvrn;
> > HOST_WIDE_INT size;
> > - bool abi_break;
> > + unsigned int abi_break;
> >
> > /* We need to do this once per argument. */
> > if (pcum->aapcs_arg_processed)
> > @@ -6322,14 +6322,19 @@ aarch64_function_arg_regno_p (unsigned r
> > static unsigned int
> > aarch64_function_arg_boundary (machine_mode mode, const_tree type)
> > {
> > - bool abi_break;
> > + unsigned int abi_break;
> > unsigned int alignment = aarch64_function_arg_alignment (mode, type,
> > &abi_break);
> > + alignment = MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> > if (abi_break & warn_psabi)
> > - inform (input_location, "parameter passing for argument of type "
> > - "%qT changed in GCC 9.1", type);
> > + {
> > + abi_break = MIN (MAX (abi_break, PARM_BOUNDARY), STACK_BOUNDARY);
> > + if (alignment != abi_break)
> > + inform (input_location, "parameter passing for argument of type "
> > + "%qT changed in GCC 9.1", type);
> > + }
> >
> > - return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> > + return alignment;
> > }
> >
> > /* Implement TARGET_GET_RAW_RESULT_MODE and TARGET_GET_RAW_ARG_MODE. */
> > @@ -16616,7 +16621,7 @@ aarch64_gimplify_va_arg_expr (tree valis
> > f_stack, NULL_TREE);
> > size = int_size_in_bytes (type);
> >
> > - bool abi_break;
> > + unsigned int abi_break;
> > align
> > = aarch64_function_arg_alignment (mode, type, &abi_break) /
> > BITS_PER_UNIT;
> >
> > --- gcc/testsuite/gcc.target/aarch64/pr91710.c.jj 2021-03-18
> > 16:42:33.529232710 +0100
> > +++ gcc/testsuite/gcc.target/aarch64/pr91710.c 2021-03-18
> > 16:42:07.271518121 +0100
> > @@ -0,0 +1,16 @@
> > +/* PR target/91710 */
> > +/* { dg-do compile } */
> > +
> > +struct S { unsigned int i:4; };
> > +
> > +unsigned int test1(struct S s) { /* { dg-bogus "parameter passing for
> > argument of type" } */
> > + return s.i;
> > +}
> > +
> > +unsigned int test2(unsigned x, struct S s) { /* { dg-bogus
> > "parameter passing for argument of type" } */
> > + return x - s.i;
> > +}
> > +
> > +unsigned int test3(unsigned x, unsigned y, struct S s) { /* { dg-bogus
> > "parameter passing for argument of type" } */
> > + return x - y - s.i;
> > +}
Jakub
