* [PATCH] mips: Fix overaligned function arguments [PR109435] @ 2023-05-29 10:32 Jovan Dmitrovic 2023-05-31 10:05 ` YunQiang Su 0 siblings, 1 reply; 10+ messages in thread From: Jovan Dmitrovic @ 2023-05-29 10:32 UTC (permalink / raw) To: gcc-patches; +Cc: djordje.todorovic, Jovan Dmitrovic, Jovan Dmitrovic This patch changes alignment for typedef types when passed as arguments, making the alignment equal to the alignment of original (aliased) types. This change makes it impossible for a typedef type to have alignment that is less than its size. Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> gcc/ChangeLog: PR target/109435 * config/mips/mips.cc (mips_function_arg_alignment): Returns the alignment of function argument. In case of typedef type, it returns the aligment of the aliased type. (mips_function_arg_boundary): Relocated calculation of the aligment of function arguments. gcc/testsuite/ChangeLog: PR target/109435 * gcc.target/mips/align-1.c: New test. --- gcc/config/mips/mips.cc | 18 +++++++++++++- gcc/testsuite/gcc.target/mips/align-1.c | 33 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc index ca822758b41..2019b7cd7d9 100644 --- a/gcc/config/mips/mips.cc +++ b/gcc/config/mips/mips.cc @@ -6190,6 +6190,22 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; } +/* Given MODE and TYPE of a function argument, return the alignment in + bits. In case of typedef, alignment of its original type is + used. */ + +static unsigned int +mips_function_arg_alignment (machine_mode mode, const_tree type) +{ + if (!type) + return GET_MODE_ALIGNMENT (mode); + + if (is_typedef_decl (TYPE_NAME (type))) + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); + + return TYPE_ALIGN (type); +} + /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at least PARM_BOUNDARY bits of alignment, but will be given anything up to STACK_BOUNDARY bits if the type requires it. */ @@ -6198,8 +6214,8 @@ static unsigned int mips_function_arg_boundary (machine_mode mode, const_tree type) { unsigned int alignment; + alignment = mips_function_arg_alignment (mode, type); - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); if (alignment < PARM_BOUNDARY) alignment = PARM_BOUNDARY; if (alignment > STACK_BOUNDARY) diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c new file mode 100644 index 00000000000..816751b8099 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1.c @@ -0,0 +1,33 @@ +/* Check that typedef alignment does not affect passing of function + parameters. */ +/* { dg-do run { target { "mips*-*-linux*" } } } */ + +#include <assert.h> + +typedef struct ui8 +{ + unsigned v[8]; +} uint8 __attribute__ ((aligned(64))); + +unsigned +callee (int x, uint8 a) +{ + return a.v[0]; +} + +uint8 +identity (uint8 in) +{ + return in; +} + +int +main (void) +{ + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; + uint8 temp = identity (vec); + unsigned temp2 = callee (1, identity (vec)); + assert (callee (1, temp) == 1); + assert (temp2 == 1); + return 0; +} -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mips: Fix overaligned function arguments [PR109435] 2023-05-29 10:32 [PATCH] mips: Fix overaligned function arguments [PR109435] Jovan Dmitrovic @ 2023-05-31 10:05 ` YunQiang Su 2023-06-06 10:29 ` Jovan Dmitrovic 0 siblings, 1 reply; 10+ messages in thread From: YunQiang Su @ 2023-05-31 10:05 UTC (permalink / raw) To: Jovan Dmitrovic; +Cc: gcc-patches, djordje.todorovic Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年5月29日周一 19:00写道: > > This patch changes alignment for typedef types when passed as > arguments, making the alignment equal to the alignment of > original (aliased) types. > > This change makes it impossible for a typedef type to have > alignment that is less than its size. > > Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> > > gcc/ChangeLog: > PR target/109435 > * config/mips/mips.cc (mips_function_arg_alignment): Returns > the alignment of function argument. In case of typedef type, > it returns the aligment of the aliased type. > (mips_function_arg_boundary): Relocated calculation of the > aligment of function arguments. > > gcc/testsuite/ChangeLog: > PR target/109435 > * gcc.target/mips/align-1.c: New test. > --- > gcc/config/mips/mips.cc | 18 +++++++++++++- > gcc/testsuite/gcc.target/mips/align-1.c | 33 +++++++++++++++++++++++++ > 2 files changed, 50 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > index ca822758b41..2019b7cd7d9 100644 > --- a/gcc/config/mips/mips.cc > +++ b/gcc/config/mips/mips.cc > @@ -6190,6 +6190,22 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) > return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; > } > > +/* Given MODE and TYPE of a function argument, return the alignment in > + bits. In case of typedef, alignment of its original type is > + used. */ > + > +static unsigned int > +mips_function_arg_alignment (machine_mode mode, const_tree type) > +{ > + if (!type) > + return GET_MODE_ALIGNMENT (mode); > + > + if (is_typedef_decl (TYPE_NAME (type))) > + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); > + > + return TYPE_ALIGN (type); > +} > + > /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at > least PARM_BOUNDARY bits of alignment, but will be given anything up > to STACK_BOUNDARY bits if the type requires it. */ > @@ -6198,8 +6214,8 @@ static unsigned int > mips_function_arg_boundary (machine_mode mode, const_tree type) > { > unsigned int alignment; > + alignment = mips_function_arg_alignment (mode, type); > > - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); > if (alignment < PARM_BOUNDARY) > alignment = PARM_BOUNDARY; > if (alignment > STACK_BOUNDARY) > diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c > new file mode 100644 > index 00000000000..816751b8099 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/align-1.c > @@ -0,0 +1,33 @@ > +/* Check that typedef alignment does not affect passing of function > + parameters. */ > +/* { dg-do run { target { "mips*-*-linux*" } } } */ Is it possible to check the result with something like scan-assembler scan-assembler-not instead of real running? > + > +#include <assert.h> > + > +typedef struct ui8 > +{ > + unsigned v[8]; > +} uint8 __attribute__ ((aligned(64))); > + > +unsigned > +callee (int x, uint8 a) > +{ > + return a.v[0]; > +} > + > +uint8 > +identity (uint8 in) > +{ > + return in; > +} > + > +int > +main (void) > +{ > + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; > + uint8 temp = identity (vec); > + unsigned temp2 = callee (1, identity (vec)); > + assert (callee (1, temp) == 1); > + assert (temp2 == 1); > + return 0; > +} > -- > 2.34.1 > -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mips: Fix overaligned function arguments [PR109435] 2023-05-31 10:05 ` YunQiang Su @ 2023-06-06 10:29 ` Jovan Dmitrovic 2023-06-06 10:50 ` YunQiang Su 0 siblings, 1 reply; 10+ messages in thread From: Jovan Dmitrovic @ 2023-06-06 10:29 UTC (permalink / raw) To: YunQiang Su; +Cc: gcc-patches, Djordje Todorovic I suppose that it is possible to check assembly. Following is part of diff before and after my patch: 29,32c29,32 < sd $6,0($2) < sd $7,8($2) < sd $8,16($2) < sd $9,24($2) --- > sd $5,0($2) > sd $6,8($2) > sd $7,16($2) > sd $8,24($2) 63,66c63,66 < sd $6,0($2) < sd $7,8($2) < sd $8,16($2) < sd $9,24($2) --- > sd $5,0($2) > sd $6,8($2) > sd $7,16($2) > sd $8,24($2) 138,141c138,141 < ld $6,64($16) < ld $7,72($16) < ld $8,80($16) < ld $9,88($16) --- > ld $5,64($16) > ld $6,72($16) > ld $7,80($16) > ld $8,88($16) 148,151c148,151 < ld $6,64($16) < ld $7,72($16) < ld $8,80($16) < ld $9,88($16) --- > ld $5,64($16) > ld $6,72($16) > ld $7,80($16) > ld $8,88($16) 167,170c167,170 < ld $6,0($16) < ld $7,8($16) < ld $8,16($16) < ld $9,24($16) --- > ld $5,0($16) > ld $6,8($16) > ld $7,16($16) > ld $8,24($16) What my patch effectively does it rearranges the data in registers when invoking a function. I don't know whether writing this testcase as an assembly check would make sense, because that would make the testcase much less readable. ________________________________________ From: YunQiang Su <wzssyqa@gmail.com> Sent: Wednesday, May 31, 2023 12:05 PM To: Jovan Dmitrovic Cc: gcc-patches@gcc.gnu.org; Djordje Todorovic Subject: Re: [PATCH] mips: Fix overaligned function arguments [PR109435] Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年5月29日周一 19:00写道: > > This patch changes alignment for typedef types when passed as > arguments, making the alignment equal to the alignment of > original (aliased) types. > > This change makes it impossible for a typedef type to have > alignment that is less than its size. > > Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> > > gcc/ChangeLog: > PR target/109435 > * config/mips/mips.cc (mips_function_arg_alignment): Returns > the alignment of function argument. In case of typedef type, > it returns the aligment of the aliased type. > (mips_function_arg_boundary): Relocated calculation of the > aligment of function arguments. > > gcc/testsuite/ChangeLog: > PR target/109435 > * gcc.target/mips/align-1.c: New test. > --- > gcc/config/mips/mips.cc | 18 +++++++++++++- > gcc/testsuite/gcc.target/mips/align-1.c | 33 +++++++++++++++++++++++++ > 2 files changed, 50 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > index ca822758b41..2019b7cd7d9 100644 > --- a/gcc/config/mips/mips.cc > +++ b/gcc/config/mips/mips.cc > @@ -6190,6 +6190,22 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) > return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; > } > > +/* Given MODE and TYPE of a function argument, return the alignment in > + bits. In case of typedef, alignment of its original type is > + used. */ > + > +static unsigned int > +mips_function_arg_alignment (machine_mode mode, const_tree type) > +{ > + if (!type) > + return GET_MODE_ALIGNMENT (mode); > + > + if (is_typedef_decl (TYPE_NAME (type))) > + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); > + > + return TYPE_ALIGN (type); > +} > + > /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at > least PARM_BOUNDARY bits of alignment, but will be given anything up > to STACK_BOUNDARY bits if the type requires it. */ > @@ -6198,8 +6214,8 @@ static unsigned int > mips_function_arg_boundary (machine_mode mode, const_tree type) > { > unsigned int alignment; > + alignment = mips_function_arg_alignment (mode, type); > > - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); > if (alignment < PARM_BOUNDARY) > alignment = PARM_BOUNDARY; > if (alignment > STACK_BOUNDARY) > diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c > new file mode 100644 > index 00000000000..816751b8099 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/align-1.c > @@ -0,0 +1,33 @@ > +/* Check that typedef alignment does not affect passing of function > + parameters. */ > +/* { dg-do run { target { "mips*-*-linux*" } } } */ Is it possible to check the result with something like scan-assembler scan-assembler-not instead of real running? > + > +#include <assert.h> > + > +typedef struct ui8 > +{ > + unsigned v[8]; > +} uint8 __attribute__ ((aligned(64))); > + > +unsigned > +callee (int x, uint8 a) > +{ > + return a.v[0]; > +} > + > +uint8 > +identity (uint8 in) > +{ > + return in; > +} > + > +int > +main (void) > +{ > + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; > + uint8 temp = identity (vec); > + unsigned temp2 = callee (1, identity (vec)); > + assert (callee (1, temp) == 1); > + assert (temp2 == 1); > + return 0; > +} > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mips: Fix overaligned function arguments [PR109435] 2023-06-06 10:29 ` Jovan Dmitrovic @ 2023-06-06 10:50 ` YunQiang Su 2023-06-07 10:28 ` Jovan Dmitrovic 0 siblings, 1 reply; 10+ messages in thread From: YunQiang Su @ 2023-06-06 10:50 UTC (permalink / raw) To: Jovan Dmitrovic; +Cc: gcc-patches, Djordje Todorovic Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年6月6日周二 18:29写道: > > I suppose that it is possible to check assembly. > Great. > Following is part of diff before and after my patch: > > 29,32c29,32 > < sd $6,0($2) > < sd $7,8($2) > < sd $8,16($2) > < sd $9,24($2) > --- > > sd $5,0($2) > > sd $6,8($2) > > sd $7,16($2) > > sd $8,24($2) > 63,66c63,66 > < sd $6,0($2) > < sd $7,8($2) > < sd $8,16($2) > < sd $9,24($2) > --- > > sd $5,0($2) > > sd $6,8($2) > > sd $7,16($2) > > sd $8,24($2) > 138,141c138,141 > < ld $6,64($16) > < ld $7,72($16) > < ld $8,80($16) > < ld $9,88($16) > --- > > ld $5,64($16) > > ld $6,72($16) > > ld $7,80($16) > > ld $8,88($16) > 148,151c148,151 > < ld $6,64($16) > < ld $7,72($16) > < ld $8,80($16) > < ld $9,88($16) > --- > > ld $5,64($16) > > ld $6,72($16) > > ld $7,80($16) > > ld $8,88($16) > 167,170c167,170 > < ld $6,0($16) > < ld $7,8($16) > < ld $8,16($16) > < ld $9,24($16) > --- > > ld $5,0($16) > > ld $6,8($16) > > ld $7,16($16) > > ld $8,24($16) > > What my patch effectively does it rearranges the data in > registers when invoking a function. I don't know whether > writing this testcase as an assembly check would make sense, > because that would make the testcase much less readable. I prefer an assembly check, because the test can be used even for cross building. It is not required, I guess. > ________________________________________ > From: YunQiang Su <wzssyqa@gmail.com> > Sent: Wednesday, May 31, 2023 12:05 PM > To: Jovan Dmitrovic > Cc: gcc-patches@gcc.gnu.org; Djordje Todorovic > Subject: Re: [PATCH] mips: Fix overaligned function arguments [PR109435] > > Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年5月29日周一 19:00写道: > > > > This patch changes alignment for typedef types when passed as > > arguments, making the alignment equal to the alignment of > > original (aliased) types. > > > > This change makes it impossible for a typedef type to have > > alignment that is less than its size. > > > > Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> > > > > gcc/ChangeLog: > > PR target/109435 > > * config/mips/mips.cc (mips_function_arg_alignment): Returns > > the alignment of function argument. In case of typedef type, > > it returns the aligment of the aliased type. > > (mips_function_arg_boundary): Relocated calculation of the > > aligment of function arguments. > > > > gcc/testsuite/ChangeLog: > > PR target/109435 > > * gcc.target/mips/align-1.c: New test. > > --- > > gcc/config/mips/mips.cc | 18 +++++++++++++- > > gcc/testsuite/gcc.target/mips/align-1.c | 33 +++++++++++++++++++++++++ > > 2 files changed, 50 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c > > > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > > index ca822758b41..2019b7cd7d9 100644 > > --- a/gcc/config/mips/mips.cc > > +++ b/gcc/config/mips/mips.cc > > @@ -6190,6 +6190,22 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) > > return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; > > } > > > > +/* Given MODE and TYPE of a function argument, return the alignment in > > + bits. In case of typedef, alignment of its original type is > > + used. */ > > + > > +static unsigned int > > +mips_function_arg_alignment (machine_mode mode, const_tree type) > > +{ > > + if (!type) > > + return GET_MODE_ALIGNMENT (mode); > > + > > + if (is_typedef_decl (TYPE_NAME (type))) > > + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); > > + > > + return TYPE_ALIGN (type); > > +} > > + > > /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at > > least PARM_BOUNDARY bits of alignment, but will be given anything up > > to STACK_BOUNDARY bits if the type requires it. */ > > @@ -6198,8 +6214,8 @@ static unsigned int > > mips_function_arg_boundary (machine_mode mode, const_tree type) > > { > > unsigned int alignment; > > + alignment = mips_function_arg_alignment (mode, type); > > > > - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); > > if (alignment < PARM_BOUNDARY) > > alignment = PARM_BOUNDARY; > > if (alignment > STACK_BOUNDARY) > > diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c > > new file mode 100644 > > index 00000000000..816751b8099 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/mips/align-1.c > > @@ -0,0 +1,33 @@ > > +/* Check that typedef alignment does not affect passing of function > > + parameters. */ > > +/* { dg-do run { target { "mips*-*-linux*" } } } */ > > Is it possible to check the result with something like > scan-assembler > scan-assembler-not > instead of real running? > > > + > > +#include <assert.h> > > + > > +typedef struct ui8 > > +{ > > + unsigned v[8]; > > +} uint8 __attribute__ ((aligned(64))); > > + > > +unsigned > > +callee (int x, uint8 a) > > +{ > > + return a.v[0]; > > +} > > + > > +uint8 > > +identity (uint8 in) > > +{ > > + return in; > > +} > > + > > +int > > +main (void) > > +{ > > + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; > > + uint8 temp = identity (vec); > > + unsigned temp2 = callee (1, identity (vec)); > > + assert (callee (1, temp) == 1); > > + assert (temp2 == 1); > > + return 0; > > +} > > -- > > 2.34.1 > > -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mips: Fix overaligned function arguments [PR109435] 2023-06-06 10:50 ` YunQiang Su @ 2023-06-07 10:28 ` Jovan Dmitrovic 2023-06-16 8:07 ` YunQiang Su 0 siblings, 1 reply; 10+ messages in thread From: Jovan Dmitrovic @ 2023-06-07 10:28 UTC (permalink / raw) To: YunQiang Su; +Cc: gcc-patches, Djordje Todorovic I see what you mean now, so I've made adjustment in order for testcase to work on assembly. Following is the updated patch. Regards, Jovan From 2744357b5232c61bf1f780c4915d47b19d71f993 Mon Sep 17 00:00:00 2001 From: Jovan Dmitrovic <Jovan.Dmitrovic@Syrmia.com> Date: Fri, 19 May 2023 12:36:55 +0200 Subject: [PATCH] mips: Fix overaligned function arguments [PR109435] This patch changes alignment for typedef types when passed as arguments, making the alignment equal to the alignment of original (aliased) types. This change makes it impossible for a typedef type to have alignment that is less than its size. Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> gcc/ChangeLog: PR target/109435 * config/mips/mips.cc (mips_function_arg_alignment): Returns the alignment of function argument. In case of typedef type, it returns the aligment of the aliased type. (mips_function_arg_boundary): Relocated calculation of the aligment of function arguments. gcc/testsuite/ChangeLog: PR target/109435 * gcc.target/mips/align-1.c: New test. --- gcc/config/mips/mips.cc | 19 ++++++++++++- gcc/testsuite/gcc.target/mips/align-1.c | 38 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc index c1d1691306e..20ba35f754c 100644 --- a/gcc/config/mips/mips.cc +++ b/gcc/config/mips/mips.cc @@ -6190,6 +6190,23 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; } +/* Given MODE and TYPE of a function argument, return the alignment in + bits. + In case of typedef, alignment of its original type is + used. */ + +static unsigned int +mips_function_arg_alignment (machine_mode mode, const_tree type) +{ + if (!type) + return GET_MODE_ALIGNMENT (mode); + + if (is_typedef_decl (TYPE_NAME (type))) + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); + + return TYPE_ALIGN (type); +} + /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at least PARM_BOUNDARY bits of alignment, but will be given anything up to STACK_BOUNDARY bits if the type requires it. */ @@ -6198,8 +6215,8 @@ static unsigned int mips_function_arg_boundary (machine_mode mode, const_tree type) { unsigned int alignment; + alignment = mips_function_arg_alignment (mode, type); - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); if (alignment < PARM_BOUNDARY) alignment = PARM_BOUNDARY; if (alignment > STACK_BOUNDARY) diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c new file mode 100644 index 00000000000..5c639bee274 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1.c @@ -0,0 +1,38 @@ +/* Check that typedef alignment does not affect passing of function + parameters. */ +/* { dg-do compile { target { "mips*-*-linux*" } } } */ +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */ + +#include <assert.h> + +typedef struct ui8 +{ + unsigned v[8]; +} uint8 __attribute__ ((aligned(64))); + +unsigned +callee (int x, uint8 a) +{ + return a.v[0]; +} + +uint8 +identity (uint8 in) +{ + return in; +} + +int +main (void) +{ + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; + uint8 temp = identity (vec); + unsigned temp2 = callee (1, identity (vec)); + assert (callee (1, temp) == 1); + assert (temp2 == 1); + return 0; +} + +/* { dg-final { scan-assembler "\tsd\t\\\$5,0\\(\\\$\[0-9\]\\)" } } */ +/* { dg-final { scan-assembler "\tsd\t\\\$6,8\\(\\\$\[0-9\]\\)" } } */ +/* { dg-final { scan-assembler "\tsd\t\\\$7,16\\(\\\$\[0-9\]\\)" } } */ -- 2.34.1 -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mips: Fix overaligned function arguments [PR109435] 2023-06-07 10:28 ` Jovan Dmitrovic @ 2023-06-16 8:07 ` YunQiang Su 2023-06-27 8:54 ` [PATCH v2] " Jovan Dmitrovic 0 siblings, 1 reply; 10+ messages in thread From: YunQiang Su @ 2023-06-16 8:07 UTC (permalink / raw) To: Jovan Dmitrovic; +Cc: gcc-patches, Djordje Todorovic Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年6月7日周三 18:29写道: > > I see what you mean now, so I've made adjustment in order for testcase to work > on assembly. Following is the updated patch. > > Regards, > Jovan > > From 2744357b5232c61bf1f780c4915d47b19d71f993 Mon Sep 17 00:00:00 2001 > From: Jovan Dmitrovic <Jovan.Dmitrovic@Syrmia.com> > Date: Fri, 19 May 2023 12:36:55 +0200 > Subject: [PATCH] mips: Fix overaligned function arguments [PR109435] > > This patch changes alignment for typedef types when passed as > arguments, making the alignment equal to the alignment of > original (aliased) types. > > This change makes it impossible for a typedef type to have > alignment that is less than its size. > > Signed-off-by: Jovan Dmitrovic <jovan.dmitrovic@syrmia.com> > > gcc/ChangeLog: > PR target/109435 > * config/mips/mips.cc (mips_function_arg_alignment): Returns > the alignment of function argument. In case of typedef type, > it returns the aligment of the aliased type. > (mips_function_arg_boundary): Relocated calculation of the > aligment of function arguments. > Please refer https://gcc.gnu.org/contribute.html about how to work with the ChangeLog. > gcc/testsuite/ChangeLog: > PR target/109435 > * gcc.target/mips/align-1.c: New test. > --- > gcc/config/mips/mips.cc | 19 ++++++++++++- > gcc/testsuite/gcc.target/mips/align-1.c | 38 +++++++++++++++++++++++++ > 2 files changed, 56 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > index c1d1691306e..20ba35f754c 100644 > --- a/gcc/config/mips/mips.cc > +++ b/gcc/config/mips/mips.cc > @@ -6190,6 +6190,23 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) > return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; > } > > +/* Given MODE and TYPE of a function argument, return the alignment in > + bits. > + In case of typedef, alignment of its original type is > + used. */ > + > +static unsigned int > +mips_function_arg_alignment (machine_mode mode, const_tree type) > +{ > + if (!type) > + return GET_MODE_ALIGNMENT (mode); > + > + if (is_typedef_decl (TYPE_NAME (type))) > + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); > + > + return TYPE_ALIGN (type); > +} > + > /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at > least PARM_BOUNDARY bits of alignment, but will be given anything up > to STACK_BOUNDARY bits if the type requires it. */ > @@ -6198,8 +6215,8 @@ static unsigned int > mips_function_arg_boundary (machine_mode mode, const_tree type) > { > unsigned int alignment; > + alignment = mips_function_arg_alignment (mode, type); > > - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); > if (alignment < PARM_BOUNDARY) > alignment = PARM_BOUNDARY; > if (alignment > STACK_BOUNDARY) > diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gcc.target/mips/align-1.c > new file mode 100644 > index 00000000000..5c639bee274 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/align-1.c > @@ -0,0 +1,38 @@ > +/* Check that typedef alignment does not affect passing of function > + parameters. */ > +/* { dg-do compile { target { "mips*-*-linux*" } } } */ mips* may be OK, since this test looks reasonable for bare metal platforms. > +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */ Does `-flto` required here? > + > +#include <assert.h> > + > +typedef struct ui8 > +{ > + unsigned v[8]; > +} uint8 __attribute__ ((aligned(64))); > + > +unsigned > +callee (int x, uint8 a) > +{ > + return a.v[0]; > +} > + > +uint8 > +identity (uint8 in) > +{ > + return in; > +} > + > +int > +main (void) > +{ > + uint8 vec = {{1, 2, 3, 4, 5, 6, 7, 8}}; > + uint8 temp = identity (vec); > + unsigned temp2 = callee (1, identity (vec)); > + assert (callee (1, temp) == 1); > + assert (temp2 == 1); > + return 0; > +} > + > +/* { dg-final { scan-assembler "\tsd\t\\\$5,0\\(\\\$\[0-9\]\\)" } } */ > +/* { dg-final { scan-assembler "\tsd\t\\\$6,8\\(\\\$\[0-9\]\\)" } } */ > +/* { dg-final { scan-assembler "\tsd\t\\\$7,16\\(\\\$\[0-9\]\\)" } } */ I guess, this test may fail for mips32 targets? Maybe we can add 2 tests: one for O32, and one for N32/N64. Add `-mabi=32`/`-mabi=n32` option into `dg-do compile` line. > -- > 2.34.1 > > > > > -- > YunQiang Su -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mips: Fix overaligned function arguments [PR109435] 2023-06-16 8:07 ` YunQiang Su @ 2023-06-27 8:54 ` Jovan Dmitrovic 2023-06-29 6:04 ` YunQiang Su 0 siblings, 1 reply; 10+ messages in thread From: Jovan Dmitrovic @ 2023-06-27 8:54 UTC (permalink / raw) To: YunQiang Su; +Cc: gcc-patches, Djordje Todorovic [-- Attachment #1: Type: text/plain, Size: 425 bytes --] Hi, I am sending a revised patch, now with different tests for N64/N32 and O32 ABIs. For the O32 ABI, I've skipped the -O0 and -Os pipelines, considering there is a difference between exact offsets for store instructions (the registers used remain the same). Skipping -flto isn't really necessary, so I've removed that part. I've fixed the Changelog, hopefully I've corrected the mistakes I made. Regards, Jovan [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-mips-Fix-overaligned-function-arguments-PR109435.patch --] [-- Type: text/x-patch; name="0001-mips-Fix-overaligned-function-arguments-PR109435.patch", Size: 4414 bytes --] From 05e4ff4d2fbb91ea8040fb10d8d6a130ad24bba7 Mon Sep 17 00:00:00 2001 From: Jovan Dmitrovic <Jovan.Dmitrovic@Syrmia.com> Date: Mon, 26 Jun 2023 17:00:20 +0200 Subject: [PATCH] mips: Fix overaligned function arguments [PR109435] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch changes alignment for typedef types when passed as arguments, making the alignment equal to the alignment of original (aliased) types. This change makes it impossible for a typedef type to have alignment that is less than its size. 2023-06-27 Jovan Dmitrović <jovan.dmitrovic@syrmia.com> gcc/ChangeLog: PR target/109435 * config/mips/mips.cc (mips_function_arg_alignment): Returns the alignment of function argument. In case of typedef type, it returns the aligment of the aliased type. (mips_function_arg_boundary): Relocated calculation of the aligment of function arguments. gcc/testsuite/ChangeLog: * gcc.target/mips/align-1-n64.c: New test. * gcc.target/mips/align-1-o32.c: New test. --- gcc/config/mips/mips.cc | 19 ++++++++++++++++++- gcc/testsuite/gcc.target/mips/align-1-n64.c | 19 +++++++++++++++++++ gcc/testsuite/gcc.target/mips/align-1-o32.c | 20 ++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/mips/align-1-n64.c create mode 100644 gcc/testsuite/gcc.target/mips/align-1-o32.c diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc index c1d1691306e..20ba35f754c 100644 --- a/gcc/config/mips/mips.cc +++ b/gcc/config/mips/mips.cc @@ -6190,6 +6190,23 @@ mips_arg_partial_bytes (cumulative_args_t cum, const function_arg_info &arg) return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; } +/* Given MODE and TYPE of a function argument, return the alignment in + bits. + In case of typedef, alignment of its original type is + used. */ + +static unsigned int +mips_function_arg_alignment (machine_mode mode, const_tree type) +{ + if (!type) + return GET_MODE_ALIGNMENT (mode); + + if (is_typedef_decl (TYPE_NAME (type))) + type = DECL_ORIGINAL_TYPE (TYPE_NAME (type)); + + return TYPE_ALIGN (type); +} + /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at least PARM_BOUNDARY bits of alignment, but will be given anything up to STACK_BOUNDARY bits if the type requires it. */ @@ -6198,8 +6215,8 @@ static unsigned int mips_function_arg_boundary (machine_mode mode, const_tree type) { unsigned int alignment; + alignment = mips_function_arg_alignment (mode, type); - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); if (alignment < PARM_BOUNDARY) alignment = PARM_BOUNDARY; if (alignment > STACK_BOUNDARY) diff --git a/gcc/testsuite/gcc.target/mips/align-1-n64.c b/gcc/testsuite/gcc.target/mips/align-1-n64.c new file mode 100644 index 00000000000..46e718d548d --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1-n64.c @@ -0,0 +1,19 @@ +/* Check that typedef alignment does not affect passing of function + parameters for N64/N32 ABIs. */ +/* { dg-do compile { target { "mips*-*-*" } } } */ +/* { dg-options "-mabi=64" } */ + +typedef struct ui8 +{ + unsigned v[8]; +} uint8 __attribute__ ((aligned(64))); + +unsigned +callee (int x, uint8 a) +{ + return a.v[0]; +} + +/* { dg-final { scan-assembler "\tsd\t\\\$5,0\\(\\\$\[0-9\]\\)" } } */ +/* { dg-final { scan-assembler "\tsd\t\\\$6,8\\(\\\$\[0-9\]\\)" } } */ +/* { dg-final { scan-assembler "\tsd\t\\\$7,16\\(\\\$\[0-9\]\\)" } } */ diff --git a/gcc/testsuite/gcc.target/mips/align-1-o32.c b/gcc/testsuite/gcc.target/mips/align-1-o32.c new file mode 100644 index 00000000000..a548632b7f6 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1-o32.c @@ -0,0 +1,20 @@ +/* Check that typedef alignment does not affect passing of function + parameters for O32 ABI. */ +/* { dg-do compile { target { "mips*-*-*" } } } */ +/* { dg-options "-mabi=32" } */ +/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" } { "" } } */ + +typedef struct ui8 +{ + unsigned v[8]; +} uint8 __attribute__ ((aligned(64))); + +unsigned +callee (int x, uint8 a) +{ + return a.v[0]; +} + +/* { dg-final { scan-assembler "\tsw\t\\\$5,100\\(\\\$sp\\)" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$6,104\\(\\\$sp\\)" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$7,108\\(\\\$sp\\)" } } */ -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mips: Fix overaligned function arguments [PR109435] 2023-06-27 8:54 ` [PATCH v2] " Jovan Dmitrovic @ 2023-06-29 6:04 ` YunQiang Su 2023-06-29 10:04 ` YunQiang Su 0 siblings, 1 reply; 10+ messages in thread From: YunQiang Su @ 2023-06-29 6:04 UTC (permalink / raw) To: Jovan Dmitrovic; +Cc: gcc-patches, Djordje Todorovic Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年6月27日周二 16:54写道: > > Hi, > I am sending a revised patch, now with different tests for N64/N32 and O32 ABIs. > For the O32 ABI, I've skipped the -O0 and -Os pipelines, considering there is a > difference between exact offsets for store instructions (the registers used remain > the same). > > Skipping -flto isn't really necessary, so I've removed that part. > > I've fixed the Changelog, hopefully I've corrected the mistakes I made. > Looks good. I will submit this patch with some format improvement. > Regards, > Jovan -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mips: Fix overaligned function arguments [PR109435] 2023-06-29 6:04 ` YunQiang Su @ 2023-06-29 10:04 ` YunQiang Su 2023-06-29 10:46 ` Jovan Dmitrovic 0 siblings, 1 reply; 10+ messages in thread From: YunQiang Su @ 2023-06-29 10:04 UTC (permalink / raw) To: Jovan Dmitrovic; +Cc: gcc-patches, Djordje Todorovic YunQiang Su <wzssyqa@gmail.com> 于2023年6月29日周四 14:04写道: > > Jovan Dmitrovic <Jovan.Dmitrovic@syrmia.com> 于2023年6月27日周二 16:54写道: > > > > Hi, > > I am sending a revised patch, now with different tests for N64/N32 and O32 ABIs. > > For the O32 ABI, I've skipped the -O0 and -Os pipelines, considering there is a > > difference between exact offsets for store instructions (the registers used remain > > the same). > > > > Skipping -flto isn't really necessary, so I've removed that part. > > > > I've fixed the Changelog, hopefully I've corrected the mistakes I made. > > > > Looks good. > I will submit this patch with some format improvement. > Ohh, my fault: the `-flto` option should always be skipped, when run test. And you skipped -O0 test on O32, while this bug effects O0 only, it should not be expected. The below is my modification to your patch. Is it OK for you? --- xx.patch 2023-06-29 14:32:59.805474033 +0800 +++ build/0001-mips-Fix-overaligned-function-arguments-PR109435.patch 2023-06-29 18:01:19.245478275 +0800 @@ -1,4 +1,4 @@ -From 05e4ff4d2fbb91ea8040fb10d8d6a130ad24bba7 Mon Sep 17 00:00:00 2001 +From 7b5af22bb7c8fadce27e94c37c96101a06acd286 Mon Sep 17 00:00:00 2001 From: Jovan Dmitrovic <Jovan.Dmitrovic@Syrmia.com> Date: Mon, 26 Jun 2023 17:00:20 +0200 Subject: [PATCH] mips: Fix overaligned function arguments [PR109435] @@ -16,12 +16,13 @@ 2023-06-27 Jovan Dmitrović <jovan.dmitrovic@syrmia.com> gcc/ChangeLog: - PR target/109435 + + PR target/109435 * config/mips/mips.cc (mips_function_arg_alignment): Returns - the alignment of function argument. In case of typedef type, - it returns the aligment of the aliased type. + the alignment of function argument. In case of typedef type, + it returns the aligment of the aliased type. (mips_function_arg_boundary): Relocated calculation of the - aligment of function arguments. + aligment of function arguments. gcc/testsuite/ChangeLog: @@ -29,9 +30,9 @@ * gcc.target/mips/align-1-o32.c: New test. --- gcc/config/mips/mips.cc | 19 ++++++++++++++++++- - gcc/testsuite/gcc.target/mips/align-1-n64.c | 19 +++++++++++++++++++ + gcc/testsuite/gcc.target/mips/align-1-n64.c | 20 ++++++++++++++++++++ gcc/testsuite/gcc.target/mips/align-1-o32.c | 20 ++++++++++++++++++++ - 3 files changed, 57 insertions(+), 1 deletion(-) + 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/mips/align-1-n64.c create mode 100644 gcc/testsuite/gcc.target/mips/align-1-o32.c @@ -75,14 +76,15 @@ if (alignment > STACK_BOUNDARY) diff --git a/gcc/testsuite/gcc.target/mips/align-1-n64.c b/gcc/testsuite/gcc.target/mips/align-1-n64.c new file mode 100644 -index 00000000000..46e718d548d +index 00000000000..3ede539c3a4 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1-n64.c -@@ -0,0 +1,19 @@ +@@ -0,0 +1,20 @@ +/* Check that typedef alignment does not affect passing of function + parameters for N64/N32 ABIs. */ +/* { dg-do compile { target { "mips*-*-*" } } } */ +/* { dg-options "-mabi=64" } */ ++/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */ + +typedef struct ui8 +{ @@ -100,7 +102,7 @@ +/* { dg-final { scan-assembler "\tsd\t\\\$7,16\\(\\\$\[0-9\]\\)" } } */ diff --git a/gcc/testsuite/gcc.target/mips/align-1-o32.c b/gcc/testsuite/gcc.target/mips/align-1-o32.c new file mode 100644 -index 00000000000..a548632b7f6 +index 00000000000..e043d6a3eca --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/align-1-o32.c @@ -0,0 +1,20 @@ @@ -108,7 +110,7 @@ + parameters for O32 ABI. */ +/* { dg-do compile { target { "mips*-*-*" } } } */ +/* { dg-options "-mabi=32" } */ -+/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" } { "" } } */ ++/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */ + +typedef struct ui8 +{ @@ -121,10 +123,9 @@ + return a.v[0]; +} + -+/* { dg-final { scan-assembler "\tsw\t\\\$5,100\\(\\\$sp\\)" } } */ -+/* { dg-final { scan-assembler "\tsw\t\\\$6,104\\(\\\$sp\\)" } } */ -+/* { dg-final { scan-assembler "\tsw\t\\\$7,108\\(\\\$sp\\)" } } */ ++/* { dg-final { scan-assembler "\tsw\t\\\$5,1\\d\\d\\(\\\$(sp|fp)\\)" } } */ ++/* { dg-final { scan-assembler "\tsw\t\\\$6,1\\d\\d\\(\\\$(sp|fp)\\)" } } */ ++/* { dg-final { scan-assembler "\tsw\t\\\$7,1\\d\\d\\(\\\$(sp|fp)\\)" } } */ -- -2.34.1 - +2.30.2 > > Regards, > > Jovan > > > > -- > YunQiang Su -- YunQiang Su ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mips: Fix overaligned function arguments [PR109435] 2023-06-29 10:04 ` YunQiang Su @ 2023-06-29 10:46 ` Jovan Dmitrovic 0 siblings, 0 replies; 10+ messages in thread From: Jovan Dmitrovic @ 2023-06-29 10:46 UTC (permalink / raw) To: YunQiang Su; +Cc: gcc-patches, Djordje Todorovic > Ohh, my fault: the `-flto` option should always be skipped, when run test. Right, if tests run with `-flto` option, they will fail. However, I do believe they are run only if LTO support is enabled, that's why my tests all passed without explicitly skipping that option. Your modification looks good to me. Regards, Jovan ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-06-29 10:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-29 10:32 [PATCH] mips: Fix overaligned function arguments [PR109435] Jovan Dmitrovic 2023-05-31 10:05 ` YunQiang Su 2023-06-06 10:29 ` Jovan Dmitrovic 2023-06-06 10:50 ` YunQiang Su 2023-06-07 10:28 ` Jovan Dmitrovic 2023-06-16 8:07 ` YunQiang Su 2023-06-27 8:54 ` [PATCH v2] " Jovan Dmitrovic 2023-06-29 6:04 ` YunQiang Su 2023-06-29 10:04 ` YunQiang Su 2023-06-29 10:46 ` Jovan Dmitrovic
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).