* [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710]
@ 2021-03-19 9:46 Jakub Jelinek
2021-03-19 13:04 ` Richard Sandiford
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-03-19 9:46 UTC (permalink / raw)
To: Richard Earnshaw, Richard Sandiford, Marcus Shawcroft, Kyrylo Tkachov
Cc: gcc-patches
Hi!
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?
2021-03-18 Jakub Jelinek <jakub@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710]
2021-03-19 9:46 [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710] Jakub Jelinek
@ 2021-03-19 13:04 ` Richard Sandiford
2021-03-29 9:16 ` Patch ping - " Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2021-03-19 13:04 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov, gcc-patches
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> 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.
Thanks,
Richard
>
> 2021-03-18 Jakub Jelinek <jakub@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Patch ping - Re: [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710]
2021-03-19 13:04 ` Richard Sandiford
@ 2021-03-29 9:16 ` Jakub Jelinek
2021-04-07 13:53 ` Aarch64 patch ping^2 Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-03-29 9:16 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches
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 <jakub@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Aarch64 patch ping^2
2021-03-29 9:16 ` Patch ping - " Jakub Jelinek
@ 2021-04-07 13:53 ` Jakub Jelinek
2021-04-14 10:16 ` Aarch64 patch ping^3 Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-04-07 13:53 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches
On Mon, Mar 29, 2021 at 11:16:55AM +0200, Jakub Jelinek wrote:
> > 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.
Ping.
> > > 2021-03-18 Jakub Jelinek <jakub@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.
Thanks.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Aarch64 patch ping^3
2021-04-07 13:53 ` Aarch64 patch ping^2 Jakub Jelinek
@ 2021-04-14 10:16 ` Jakub Jelinek
2021-04-16 17:55 ` Richard Sandiford
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-04-14 10:16 UTC (permalink / raw)
To: Richard Earnshaw, gcc-patches
On Wed, Apr 07, 2021 at 03:53:26PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Mon, Mar 29, 2021 at 11:16:55AM +0200, Jakub Jelinek wrote:
> > > 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.
>
> Ping.
Ping.
> > > > 2021-03-18 Jakub Jelinek <jakub@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.
Thanks
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Aarch64 patch ping^3
2021-04-14 10:16 ` Aarch64 patch ping^3 Jakub Jelinek
@ 2021-04-16 17:55 ` Richard Sandiford
0 siblings, 0 replies; 6+ messages in thread
From: Richard Sandiford @ 2021-04-16 17:55 UTC (permalink / raw)
To: Jakub Jelinek via Gcc-patches; +Cc: Richard Earnshaw, Jakub Jelinek
Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Wed, Apr 07, 2021 at 03:53:26PM +0200, Jakub Jelinek via Gcc-patches wrote:
>> On Mon, Mar 29, 2021 at 11:16:55AM +0200, Jakub Jelinek wrote:
>> > > 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.
>>
>> Ping.
>
> Ping.
Let's go for it. I still think the patch looks good, and if a problem
crops up then we still have time to fix it before the release. Applying
it now seems better than applying it closer to the release.
Thanks,
Richard
>
>> > > > 2021-03-18 Jakub Jelinek <jakub@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.
>
> Thanks
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-16 17:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 9:46 [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710] Jakub Jelinek
2021-03-19 13:04 ` Richard Sandiford
2021-03-29 9:16 ` Patch ping - " Jakub Jelinek
2021-04-07 13:53 ` Aarch64 patch ping^2 Jakub Jelinek
2021-04-14 10:16 ` Aarch64 patch ping^3 Jakub Jelinek
2021-04-16 17:55 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).