| Message ID | 20240219091306.523967-1-torbjorn.svensson@foss.st.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Fixed C23 call compatibility with arm-none-eabi | expand |
On Mon, Feb 19, 2024 at 1:14 AM Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> wrote: > > Ok for trunk and releases/gcc-13? > Regtested on top of 945cb8490cb for arm-none-eabi, without any regression. > > Backporting to releases/gcc-13 will change -std=c23 to -std=c2x. > > -- > > In commit 4fe34cdcc80ac225b80670eabc38ac5e31ce8a5a, -std=c23 support was > introduced to support functions without any named arguments. For > arm-none-eabi, this is not as simple as placing all arguments on the > stack. Align the caller to use r0, r1, r2 and r3 for arguments even for > functions without any named arguments, as specified in the AAPCS. > > Verify that the generic test case have the arguments are in the right > order and add ARM specific test cases. > > gcc/ChangeLog: > > * calls.h: Added the type of the function to function_arg_info. > * calls.cc: Save the type of the function. > * config/arm/arm.cc: Check in the AAPCS layout function if > function has no named args. > > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/c23-stdarg-split-1a.c: Detect out of order > arguments. > * gcc.dg/torture/c23-stdarg-split-1b.c: Likewise. It is almost always better to add a new testcase for the expanded idea of the test rather than modifying the original. Thanks, Andrew Pinski > * gcc.target/arm/aapcs/align_vaarg3.c: New test. > * gcc.target/arm/aapcs/align_vaarg4.c: New test. > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> > Co-authored-by: Yvan ROUX <yvan.roux@foss.st.com> > --- > gcc/calls.cc | 2 +- > gcc/calls.h | 20 ++++++++-- > gcc/config/arm/arm.cc | 13 ++++--- > .../gcc.dg/torture/c23-stdarg-split-1a.c | 4 +- > .../gcc.dg/torture/c23-stdarg-split-1b.c | 15 +++++--- > .../gcc.target/arm/aapcs/align_vaarg3.c | 37 +++++++++++++++++++ > .../gcc.target/arm/aapcs/align_vaarg4.c | 31 ++++++++++++++++ > 7 files changed, 102 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > > diff --git a/gcc/calls.cc b/gcc/calls.cc > index 01f44734743..a1cc283b952 100644 > --- a/gcc/calls.cc > +++ b/gcc/calls.cc > @@ -1376,7 +1376,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, > with those made by function.cc. */ > > /* See if this argument should be passed by invisible reference. */ > - function_arg_info arg (type, argpos < n_named_args); > + function_arg_info arg (type, fntype, argpos < n_named_args); > if (pass_by_reference (args_so_far_pnt, arg)) > { > const bool callee_copies > diff --git a/gcc/calls.h b/gcc/calls.h > index 464a4e34e33..88836559ebe 100644 > --- a/gcc/calls.h > +++ b/gcc/calls.h > @@ -35,24 +35,33 @@ class function_arg_info > { > public: > function_arg_info () > - : type (NULL_TREE), mode (VOIDmode), named (false), > + : type (NULL_TREE), fntype (NULL_TREE), mode (VOIDmode), named (false), > pass_by_reference (false) > {} > > /* Initialize an argument of mode MODE, either before or after promotion. */ > function_arg_info (machine_mode mode, bool named) > - : type (NULL_TREE), mode (mode), named (named), pass_by_reference (false) > + : type (NULL_TREE), fntype (NULL_TREE), mode (mode), named (named), > + pass_by_reference (false) > {} > > /* Initialize an unpromoted argument of type TYPE. */ > function_arg_info (tree type, bool named) > - : type (type), mode (TYPE_MODE (type)), named (named), > + : type (type), fntype (NULL_TREE), mode (TYPE_MODE (type)), named (named), > pass_by_reference (false) > {} > > + /* Initialize an unpromoted argument of type TYPE with a known function type > + FNTYPE. */ > + function_arg_info (tree type, tree fntype, bool named) > + : type (type), fntype (fntype), mode (TYPE_MODE (type)), named (named), > + pass_by_reference (false) > + {} > + > /* Initialize an argument with explicit properties. */ > function_arg_info (tree type, machine_mode mode, bool named) > - : type (type), mode (mode), named (named), pass_by_reference (false) > + : type (type), fntype (NULL_TREE), mode (mode), named (named), > + pass_by_reference (false) > {} > > /* Return true if the gimple-level type is an aggregate. */ > @@ -96,6 +105,9 @@ public: > libgcc support functions). */ > tree type; > > + /* The type of the function that has this argument, or null if not known. */ > + tree fntype; > + > /* The mode of the argument. Depending on context, this might be > the mode of the argument type or the mode after promotion. */ > machine_mode mode; > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > index 1cd69268ee9..98e149e5b7e 100644 > --- a/gcc/config/arm/arm.cc > +++ b/gcc/config/arm/arm.cc > @@ -7006,7 +7006,7 @@ aapcs_libcall_value (machine_mode mode) > numbers referred to here are those in the AAPCS. */ > static void > aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, > - const_tree type, bool named) > + const_tree type, bool named, const_tree fntype) > { > int nregs, nregs2; > int ncrn; > @@ -7018,8 +7018,9 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, > pcum->aapcs_arg_processed = true; > > /* Special case: if named is false then we are handling an incoming > - anonymous argument which is on the stack. */ > - if (!named) > + anonymous argument which is on the stack unless the function has no named > + arguments (functions without named arguments was introduced in C23). */ > + if ((fntype == NULL_TREE || !TYPE_NO_NAMED_ARGS_STDARG_P (fntype)) && !named) > return; > > /* Is this a potential co-processor register candidate? */ > @@ -7267,7 +7268,7 @@ arm_function_arg (cumulative_args_t pcum_v, const function_arg_info &arg) > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > return pcum->aapcs_reg; > } > > @@ -7342,7 +7343,7 @@ arm_arg_partial_bytes (cumulative_args_t pcum_v, const function_arg_info &arg) > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > return pcum->aapcs_partial; > } > > @@ -7367,7 +7368,7 @@ arm_function_arg_advance (cumulative_args_t pcum_v, > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > > if (pcum->aapcs_cprc_slot >= 0) > { > diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > index 2dff79235b2..717fc4bfd07 100644 > --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > @@ -5,7 +5,6 @@ > /* { dg-options "-std=c23 -pedantic-errors" } */ > /* { dg-additional-sources "c23-stdarg-split-1b.c" } */ > > -extern void abort (void); > extern void exit (int); > > double f (...); > @@ -22,8 +21,7 @@ void h7 (volatile struct s x, ...); > int > main () > { > - if (f (1, 2.0, 3, 4.0) != 10.0) > - abort (); > + f (1, 2.0, 3, 4.0); > g (0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); > g (0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f); > h1 (0, 0.0, 1, 2.0, 3, 4.0, 5, 6.0, 7, 8.0, 9); > diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > index 064da121ec2..3b70fe1595a 100644 > --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > @@ -8,17 +8,20 @@ > > extern void abort (void); > > -double > +void > f (...) > { > va_list ap; > va_start (ap); > - double ret = va_arg (ap, int); > - ret += va_arg (ap, double); > - ret += va_arg (ap, int); > - ret += va_arg (ap, double); > + if (va_arg (ap, int) != 1) > + abort(); > + if (va_arg (ap, double) != 2) > + abort(); > + if (va_arg (ap, int) != 3) > + abort(); > + if (va_arg (ap, double) != 4) > + abort(); > va_end (ap); > - return ret; > } > > void > diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > new file mode 100644 > index 00000000000..23cc78d5230 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > @@ -0,0 +1,37 @@ > +/* Test AAPCS layout (alignment of varargs) for callee. */ > + > +/* { dg-do run { target arm_eabi } } */ > +/* { dg-require-effective-target arm32 } */ > +/* { dg-options "-std=c23 -O2 -fno-inline" } */ > + > +#include <stdarg.h> > + > +extern void abort (void); > + > +typedef __attribute__((aligned (8))) int alignedint; > + > +void > +foo (...) > +{ > + int i = 0; > + va_list va; > + va_start (va); > + /* Arguments should be passed in the same registers as if they were ints. */ > + for (i = 5; i >= 0; i--) > + if (va_arg (va, int) != i) > + abort (); > + va_end (va); > +} > + > +int > +main (int argc, char **argv) > +{ > + alignedint a = 5; > + alignedint b = 4; > + alignedint c = 3; > + alignedint d = 2; > + alignedint e = 1; > + alignedint f = 0; > + foo (a, b, c, d, e, f); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > new file mode 100644 > index 00000000000..c4333f33677 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > @@ -0,0 +1,31 @@ > +/* Test AAPCS layout (alignment of varargs) for callee. */ > + > +/* { dg-do run { target arm_eabi } } */ > +/* { dg-require-effective-target arm32 } */ > +/* { dg-options "-std=c23 -O2 -fno-inline" } */ > + > +#include <stdarg.h> > + > +extern void abort (void); > + > +typedef __attribute__((aligned (8))) int alignedint; > + > +void > +foo (...) > +{ > + int i = 0; > + va_list va; > + va_start (va); > + /* alignedint should be pulled out of regs/stack just like an int. */ > + for (i = 5; i >= 0; i--) > + if (va_arg (va, alignedint) != i) > + abort (); > + va_end (va); > +} > + > +int > +main (int argc, char **argv) > +{ > + foo (5, 4, 3, 2, 1, 0); > + return 0; > +} > -- > 2.25.1 >
On 19/02/2024 09:13, Torbjörn SVENSSON wrote: > Ok for trunk and releases/gcc-13? > Regtested on top of 945cb8490cb for arm-none-eabi, without any regression. > > Backporting to releases/gcc-13 will change -std=c23 to -std=c2x. Jakub has just pushed a different fix for this, so I don't think we need this now. R. > > -- > > In commit 4fe34cdcc80ac225b80670eabc38ac5e31ce8a5a, -std=c23 support was > introduced to support functions without any named arguments. For > arm-none-eabi, this is not as simple as placing all arguments on the > stack. Align the caller to use r0, r1, r2 and r3 for arguments even for > functions without any named arguments, as specified in the AAPCS. > > Verify that the generic test case have the arguments are in the right > order and add ARM specific test cases. > > gcc/ChangeLog: > > * calls.h: Added the type of the function to function_arg_info. > * calls.cc: Save the type of the function. > * config/arm/arm.cc: Check in the AAPCS layout function if > function has no named args. > > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/c23-stdarg-split-1a.c: Detect out of order > arguments. > * gcc.dg/torture/c23-stdarg-split-1b.c: Likewise. > * gcc.target/arm/aapcs/align_vaarg3.c: New test. > * gcc.target/arm/aapcs/align_vaarg4.c: New test. > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> > Co-authored-by: Yvan ROUX <yvan.roux@foss.st.com> > --- > gcc/calls.cc | 2 +- > gcc/calls.h | 20 ++++++++-- > gcc/config/arm/arm.cc | 13 ++++--- > .../gcc.dg/torture/c23-stdarg-split-1a.c | 4 +- > .../gcc.dg/torture/c23-stdarg-split-1b.c | 15 +++++--- > .../gcc.target/arm/aapcs/align_vaarg3.c | 37 +++++++++++++++++++ > .../gcc.target/arm/aapcs/align_vaarg4.c | 31 ++++++++++++++++ > 7 files changed, 102 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > > diff --git a/gcc/calls.cc b/gcc/calls.cc > index 01f44734743..a1cc283b952 100644 > --- a/gcc/calls.cc > +++ b/gcc/calls.cc > @@ -1376,7 +1376,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, > with those made by function.cc. */ > > /* See if this argument should be passed by invisible reference. */ > - function_arg_info arg (type, argpos < n_named_args); > + function_arg_info arg (type, fntype, argpos < n_named_args); > if (pass_by_reference (args_so_far_pnt, arg)) > { > const bool callee_copies > diff --git a/gcc/calls.h b/gcc/calls.h > index 464a4e34e33..88836559ebe 100644 > --- a/gcc/calls.h > +++ b/gcc/calls.h > @@ -35,24 +35,33 @@ class function_arg_info > { > public: > function_arg_info () > - : type (NULL_TREE), mode (VOIDmode), named (false), > + : type (NULL_TREE), fntype (NULL_TREE), mode (VOIDmode), named (false), > pass_by_reference (false) > {} > > /* Initialize an argument of mode MODE, either before or after promotion. */ > function_arg_info (machine_mode mode, bool named) > - : type (NULL_TREE), mode (mode), named (named), pass_by_reference (false) > + : type (NULL_TREE), fntype (NULL_TREE), mode (mode), named (named), > + pass_by_reference (false) > {} > > /* Initialize an unpromoted argument of type TYPE. */ > function_arg_info (tree type, bool named) > - : type (type), mode (TYPE_MODE (type)), named (named), > + : type (type), fntype (NULL_TREE), mode (TYPE_MODE (type)), named (named), > pass_by_reference (false) > {} > > + /* Initialize an unpromoted argument of type TYPE with a known function type > + FNTYPE. */ > + function_arg_info (tree type, tree fntype, bool named) > + : type (type), fntype (fntype), mode (TYPE_MODE (type)), named (named), > + pass_by_reference (false) > + {} > + > /* Initialize an argument with explicit properties. */ > function_arg_info (tree type, machine_mode mode, bool named) > - : type (type), mode (mode), named (named), pass_by_reference (false) > + : type (type), fntype (NULL_TREE), mode (mode), named (named), > + pass_by_reference (false) > {} > > /* Return true if the gimple-level type is an aggregate. */ > @@ -96,6 +105,9 @@ public: > libgcc support functions). */ > tree type; > > + /* The type of the function that has this argument, or null if not known. */ > + tree fntype; > + > /* The mode of the argument. Depending on context, this might be > the mode of the argument type or the mode after promotion. */ > machine_mode mode; > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > index 1cd69268ee9..98e149e5b7e 100644 > --- a/gcc/config/arm/arm.cc > +++ b/gcc/config/arm/arm.cc > @@ -7006,7 +7006,7 @@ aapcs_libcall_value (machine_mode mode) > numbers referred to here are those in the AAPCS. */ > static void > aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, > - const_tree type, bool named) > + const_tree type, bool named, const_tree fntype) > { > int nregs, nregs2; > int ncrn; > @@ -7018,8 +7018,9 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, > pcum->aapcs_arg_processed = true; > > /* Special case: if named is false then we are handling an incoming > - anonymous argument which is on the stack. */ > - if (!named) > + anonymous argument which is on the stack unless the function has no named > + arguments (functions without named arguments was introduced in C23). */ > + if ((fntype == NULL_TREE || !TYPE_NO_NAMED_ARGS_STDARG_P (fntype)) && !named) > return; > > /* Is this a potential co-processor register candidate? */ > @@ -7267,7 +7268,7 @@ arm_function_arg (cumulative_args_t pcum_v, const function_arg_info &arg) > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > return pcum->aapcs_reg; > } > > @@ -7342,7 +7343,7 @@ arm_arg_partial_bytes (cumulative_args_t pcum_v, const function_arg_info &arg) > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > return pcum->aapcs_partial; > } > > @@ -7367,7 +7368,7 @@ arm_function_arg_advance (cumulative_args_t pcum_v, > > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); > + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); > > if (pcum->aapcs_cprc_slot >= 0) > { > diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > index 2dff79235b2..717fc4bfd07 100644 > --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c > @@ -5,7 +5,6 @@ > /* { dg-options "-std=c23 -pedantic-errors" } */ > /* { dg-additional-sources "c23-stdarg-split-1b.c" } */ > > -extern void abort (void); > extern void exit (int); > > double f (...); > @@ -22,8 +21,7 @@ void h7 (volatile struct s x, ...); > int > main () > { > - if (f (1, 2.0, 3, 4.0) != 10.0) > - abort (); > + f (1, 2.0, 3, 4.0); > g (0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); > g (0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f); > h1 (0, 0.0, 1, 2.0, 3, 4.0, 5, 6.0, 7, 8.0, 9); > diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > index 064da121ec2..3b70fe1595a 100644 > --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c > @@ -8,17 +8,20 @@ > > extern void abort (void); > > -double > +void > f (...) > { > va_list ap; > va_start (ap); > - double ret = va_arg (ap, int); > - ret += va_arg (ap, double); > - ret += va_arg (ap, int); > - ret += va_arg (ap, double); > + if (va_arg (ap, int) != 1) > + abort(); > + if (va_arg (ap, double) != 2) > + abort(); > + if (va_arg (ap, int) != 3) > + abort(); > + if (va_arg (ap, double) != 4) > + abort(); > va_end (ap); > - return ret; > } > > void > diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > new file mode 100644 > index 00000000000..23cc78d5230 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c > @@ -0,0 +1,37 @@ > +/* Test AAPCS layout (alignment of varargs) for callee. */ > + > +/* { dg-do run { target arm_eabi } } */ > +/* { dg-require-effective-target arm32 } */ > +/* { dg-options "-std=c23 -O2 -fno-inline" } */ > + > +#include <stdarg.h> > + > +extern void abort (void); > + > +typedef __attribute__((aligned (8))) int alignedint; > + > +void > +foo (...) > +{ > + int i = 0; > + va_list va; > + va_start (va); > + /* Arguments should be passed in the same registers as if they were ints. */ > + for (i = 5; i >= 0; i--) > + if (va_arg (va, int) != i) > + abort (); > + va_end (va); > +} > + > +int > +main (int argc, char **argv) > +{ > + alignedint a = 5; > + alignedint b = 4; > + alignedint c = 3; > + alignedint d = 2; > + alignedint e = 1; > + alignedint f = 0; > + foo (a, b, c, d, e, f); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > new file mode 100644 > index 00000000000..c4333f33677 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c > @@ -0,0 +1,31 @@ > +/* Test AAPCS layout (alignment of varargs) for callee. */ > + > +/* { dg-do run { target arm_eabi } } */ > +/* { dg-require-effective-target arm32 } */ > +/* { dg-options "-std=c23 -O2 -fno-inline" } */ > + > +#include <stdarg.h> > + > +extern void abort (void); > + > +typedef __attribute__((aligned (8))) int alignedint; > + > +void > +foo (...) > +{ > + int i = 0; > + va_list va; > + va_start (va); > + /* alignedint should be pulled out of regs/stack just like an int. */ > + for (i = 5; i >= 0; i--) > + if (va_arg (va, alignedint) != i) > + abort (); > + va_end (va); > +} > + > +int > +main (int argc, char **argv) > +{ > + foo (5, 4, 3, 2, 1, 0); > + return 0; > +}
On 2024-03-01 15:58, Richard Earnshaw (lists) wrote: > On 19/02/2024 09:13, Torbjörn SVENSSON wrote: >> Ok for trunk and releases/gcc-13? >> Regtested on top of 945cb8490cb for arm-none-eabi, without any regression. >> >> Backporting to releases/gcc-13 will change -std=c23 to -std=c2x. > > Jakub has just pushed a different fix for this, so I don't think we need this now. > > R. Would it still be benificial to have the 2 test cases for the AAPCS validation? Kind regards, Torbjörn > > >> >> -- >> >> In commit 4fe34cdcc80ac225b80670eabc38ac5e31ce8a5a, -std=c23 support was >> introduced to support functions without any named arguments. For >> arm-none-eabi, this is not as simple as placing all arguments on the >> stack. Align the caller to use r0, r1, r2 and r3 for arguments even for >> functions without any named arguments, as specified in the AAPCS. >> >> Verify that the generic test case have the arguments are in the right >> order and add ARM specific test cases. >> >> gcc/ChangeLog: >> >> * calls.h: Added the type of the function to function_arg_info. >> * calls.cc: Save the type of the function. >> * config/arm/arm.cc: Check in the AAPCS layout function if >> function has no named args. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.dg/torture/c23-stdarg-split-1a.c: Detect out of order >> arguments. >> * gcc.dg/torture/c23-stdarg-split-1b.c: Likewise. >> * gcc.target/arm/aapcs/align_vaarg3.c: New test. >> * gcc.target/arm/aapcs/align_vaarg4.c: New test. >> >> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> >> Co-authored-by: Yvan ROUX <yvan.roux@foss.st.com> >> --- >> gcc/calls.cc | 2 +- >> gcc/calls.h | 20 ++++++++-- >> gcc/config/arm/arm.cc | 13 ++++--- >> .../gcc.dg/torture/c23-stdarg-split-1a.c | 4 +- >> .../gcc.dg/torture/c23-stdarg-split-1b.c | 15 +++++--- >> .../gcc.target/arm/aapcs/align_vaarg3.c | 37 +++++++++++++++++++ >> .../gcc.target/arm/aapcs/align_vaarg4.c | 31 ++++++++++++++++ >> 7 files changed, 102 insertions(+), 20 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c >> create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c >> >> diff --git a/gcc/calls.cc b/gcc/calls.cc >> index 01f44734743..a1cc283b952 100644 >> --- a/gcc/calls.cc >> +++ b/gcc/calls.cc >> @@ -1376,7 +1376,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, >> with those made by function.cc. */ >> >> /* See if this argument should be passed by invisible reference. */ >> - function_arg_info arg (type, argpos < n_named_args); >> + function_arg_info arg (type, fntype, argpos < n_named_args); >> if (pass_by_reference (args_so_far_pnt, arg)) >> { >> const bool callee_copies >> diff --git a/gcc/calls.h b/gcc/calls.h >> index 464a4e34e33..88836559ebe 100644 >> --- a/gcc/calls.h >> +++ b/gcc/calls.h >> @@ -35,24 +35,33 @@ class function_arg_info >> { >> public: >> function_arg_info () >> - : type (NULL_TREE), mode (VOIDmode), named (false), >> + : type (NULL_TREE), fntype (NULL_TREE), mode (VOIDmode), named (false), >> pass_by_reference (false) >> {} >> >> /* Initialize an argument of mode MODE, either before or after promotion. */ >> function_arg_info (machine_mode mode, bool named) >> - : type (NULL_TREE), mode (mode), named (named), pass_by_reference (false) >> + : type (NULL_TREE), fntype (NULL_TREE), mode (mode), named (named), >> + pass_by_reference (false) >> {} >> >> /* Initialize an unpromoted argument of type TYPE. */ >> function_arg_info (tree type, bool named) >> - : type (type), mode (TYPE_MODE (type)), named (named), >> + : type (type), fntype (NULL_TREE), mode (TYPE_MODE (type)), named (named), >> pass_by_reference (false) >> {} >> >> + /* Initialize an unpromoted argument of type TYPE with a known function type >> + FNTYPE. */ >> + function_arg_info (tree type, tree fntype, bool named) >> + : type (type), fntype (fntype), mode (TYPE_MODE (type)), named (named), >> + pass_by_reference (false) >> + {} >> + >> /* Initialize an argument with explicit properties. */ >> function_arg_info (tree type, machine_mode mode, bool named) >> - : type (type), mode (mode), named (named), pass_by_reference (false) >> + : type (type), fntype (NULL_TREE), mode (mode), named (named), >> + pass_by_reference (false) >> {} >> >> /* Return true if the gimple-level type is an aggregate. */ >> @@ -96,6 +105,9 @@ public: >> libgcc support functions). */ >> tree type; >> >> + /* The type of the function that has this argument, or null if not known. */ >> + tree fntype; >> + >> /* The mode of the argument. Depending on context, this might be >> the mode of the argument type or the mode after promotion. */ >> machine_mode mode; >> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc >> index 1cd69268ee9..98e149e5b7e 100644 >> --- a/gcc/config/arm/arm.cc >> +++ b/gcc/config/arm/arm.cc >> @@ -7006,7 +7006,7 @@ aapcs_libcall_value (machine_mode mode) >> numbers referred to here are those in the AAPCS. */ >> static void >> aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, >> - const_tree type, bool named) >> + const_tree type, bool named, const_tree fntype) >> { >> int nregs, nregs2; >> int ncrn; >> @@ -7018,8 +7018,9 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, >> pcum->aapcs_arg_processed = true; >> >> /* Special case: if named is false then we are handling an incoming >> - anonymous argument which is on the stack. */ >> - if (!named) >> + anonymous argument which is on the stack unless the function has no named >> + arguments (functions without named arguments was introduced in C23). */ >> + if ((fntype == NULL_TREE || !TYPE_NO_NAMED_ARGS_STDARG_P (fntype)) && !named) >> return; >> >> /* Is this a potential co-processor register candidate? */ >> @@ -7267,7 +7268,7 @@ arm_function_arg (cumulative_args_t pcum_v, const function_arg_info &arg) >> >> if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) >> { >> - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); >> + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); >> return pcum->aapcs_reg; >> } >> >> @@ -7342,7 +7343,7 @@ arm_arg_partial_bytes (cumulative_args_t pcum_v, const function_arg_info &arg) >> >> if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) >> { >> - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); >> + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); >> return pcum->aapcs_partial; >> } >> >> @@ -7367,7 +7368,7 @@ arm_function_arg_advance (cumulative_args_t pcum_v, >> >> if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) >> { >> - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); >> + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); >> >> if (pcum->aapcs_cprc_slot >= 0) >> { >> diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c >> index 2dff79235b2..717fc4bfd07 100644 >> --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c >> +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c >> @@ -5,7 +5,6 @@ >> /* { dg-options "-std=c23 -pedantic-errors" } */ >> /* { dg-additional-sources "c23-stdarg-split-1b.c" } */ >> >> -extern void abort (void); >> extern void exit (int); >> >> double f (...); >> @@ -22,8 +21,7 @@ void h7 (volatile struct s x, ...); >> int >> main () >> { >> - if (f (1, 2.0, 3, 4.0) != 10.0) >> - abort (); >> + f (1, 2.0, 3, 4.0); >> g (0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); >> g (0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f); >> h1 (0, 0.0, 1, 2.0, 3, 4.0, 5, 6.0, 7, 8.0, 9); >> diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c >> index 064da121ec2..3b70fe1595a 100644 >> --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c >> +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c >> @@ -8,17 +8,20 @@ >> >> extern void abort (void); >> >> -double >> +void >> f (...) >> { >> va_list ap; >> va_start (ap); >> - double ret = va_arg (ap, int); >> - ret += va_arg (ap, double); >> - ret += va_arg (ap, int); >> - ret += va_arg (ap, double); >> + if (va_arg (ap, int) != 1) >> + abort(); >> + if (va_arg (ap, double) != 2) >> + abort(); >> + if (va_arg (ap, int) != 3) >> + abort(); >> + if (va_arg (ap, double) != 4) >> + abort(); >> va_end (ap); >> - return ret; >> } >> >> void >> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c >> new file mode 100644 >> index 00000000000..23cc78d5230 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c >> @@ -0,0 +1,37 @@ >> +/* Test AAPCS layout (alignment of varargs) for callee. */ >> + >> +/* { dg-do run { target arm_eabi } } */ >> +/* { dg-require-effective-target arm32 } */ >> +/* { dg-options "-std=c23 -O2 -fno-inline" } */ >> + >> +#include <stdarg.h> >> + >> +extern void abort (void); >> + >> +typedef __attribute__((aligned (8))) int alignedint; >> + >> +void >> +foo (...) >> +{ >> + int i = 0; >> + va_list va; >> + va_start (va); >> + /* Arguments should be passed in the same registers as if they were ints. */ >> + for (i = 5; i >= 0; i--) >> + if (va_arg (va, int) != i) >> + abort (); >> + va_end (va); >> +} >> + >> +int >> +main (int argc, char **argv) >> +{ >> + alignedint a = 5; >> + alignedint b = 4; >> + alignedint c = 3; >> + alignedint d = 2; >> + alignedint e = 1; >> + alignedint f = 0; >> + foo (a, b, c, d, e, f); >> + return 0; >> +} >> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c >> new file mode 100644 >> index 00000000000..c4333f33677 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c >> @@ -0,0 +1,31 @@ >> +/* Test AAPCS layout (alignment of varargs) for callee. */ >> + >> +/* { dg-do run { target arm_eabi } } */ >> +/* { dg-require-effective-target arm32 } */ >> +/* { dg-options "-std=c23 -O2 -fno-inline" } */ >> + >> +#include <stdarg.h> >> + >> +extern void abort (void); >> + >> +typedef __attribute__((aligned (8))) int alignedint; >> + >> +void >> +foo (...) >> +{ >> + int i = 0; >> + va_list va; >> + va_start (va); >> + /* alignedint should be pulled out of regs/stack just like an int. */ >> + for (i = 5; i >= 0; i--) >> + if (va_arg (va, alignedint) != i) >> + abort (); >> + va_end (va); >> +} >> + >> +int >> +main (int argc, char **argv) >> +{ >> + foo (5, 4, 3, 2, 1, 0); >> + return 0; >> +} >
diff --git a/gcc/calls.cc b/gcc/calls.cc index 01f44734743..a1cc283b952 100644 --- a/gcc/calls.cc +++ b/gcc/calls.cc @@ -1376,7 +1376,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, with those made by function.cc. */ /* See if this argument should be passed by invisible reference. */ - function_arg_info arg (type, argpos < n_named_args); + function_arg_info arg (type, fntype, argpos < n_named_args); if (pass_by_reference (args_so_far_pnt, arg)) { const bool callee_copies diff --git a/gcc/calls.h b/gcc/calls.h index 464a4e34e33..88836559ebe 100644 --- a/gcc/calls.h +++ b/gcc/calls.h @@ -35,24 +35,33 @@ class function_arg_info { public: function_arg_info () - : type (NULL_TREE), mode (VOIDmode), named (false), + : type (NULL_TREE), fntype (NULL_TREE), mode (VOIDmode), named (false), pass_by_reference (false) {} /* Initialize an argument of mode MODE, either before or after promotion. */ function_arg_info (machine_mode mode, bool named) - : type (NULL_TREE), mode (mode), named (named), pass_by_reference (false) + : type (NULL_TREE), fntype (NULL_TREE), mode (mode), named (named), + pass_by_reference (false) {} /* Initialize an unpromoted argument of type TYPE. */ function_arg_info (tree type, bool named) - : type (type), mode (TYPE_MODE (type)), named (named), + : type (type), fntype (NULL_TREE), mode (TYPE_MODE (type)), named (named), pass_by_reference (false) {} + /* Initialize an unpromoted argument of type TYPE with a known function type + FNTYPE. */ + function_arg_info (tree type, tree fntype, bool named) + : type (type), fntype (fntype), mode (TYPE_MODE (type)), named (named), + pass_by_reference (false) + {} + /* Initialize an argument with explicit properties. */ function_arg_info (tree type, machine_mode mode, bool named) - : type (type), mode (mode), named (named), pass_by_reference (false) + : type (type), fntype (NULL_TREE), mode (mode), named (named), + pass_by_reference (false) {} /* Return true if the gimple-level type is an aggregate. */ @@ -96,6 +105,9 @@ public: libgcc support functions). */ tree type; + /* The type of the function that has this argument, or null if not known. */ + tree fntype; + /* The mode of the argument. Depending on context, this might be the mode of the argument type or the mode after promotion. */ machine_mode mode; diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index 1cd69268ee9..98e149e5b7e 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -7006,7 +7006,7 @@ aapcs_libcall_value (machine_mode mode) numbers referred to here are those in the AAPCS. */ static void aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, - const_tree type, bool named) + const_tree type, bool named, const_tree fntype) { int nregs, nregs2; int ncrn; @@ -7018,8 +7018,9 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, machine_mode mode, pcum->aapcs_arg_processed = true; /* Special case: if named is false then we are handling an incoming - anonymous argument which is on the stack. */ - if (!named) + anonymous argument which is on the stack unless the function has no named + arguments (functions without named arguments was introduced in C23). */ + if ((fntype == NULL_TREE || !TYPE_NO_NAMED_ARGS_STDARG_P (fntype)) && !named) return; /* Is this a potential co-processor register candidate? */ @@ -7267,7 +7268,7 @@ arm_function_arg (cumulative_args_t pcum_v, const function_arg_info &arg) if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) { - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); return pcum->aapcs_reg; } @@ -7342,7 +7343,7 @@ arm_arg_partial_bytes (cumulative_args_t pcum_v, const function_arg_info &arg) if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) { - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); return pcum->aapcs_partial; } @@ -7367,7 +7368,7 @@ arm_function_arg_advance (cumulative_args_t pcum_v, if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) { - aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named); + aapcs_layout_arg (pcum, arg.mode, arg.type, arg.named, arg.fntype); if (pcum->aapcs_cprc_slot >= 0) { diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c index 2dff79235b2..717fc4bfd07 100644 --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c @@ -5,7 +5,6 @@ /* { dg-options "-std=c23 -pedantic-errors" } */ /* { dg-additional-sources "c23-stdarg-split-1b.c" } */ -extern void abort (void); extern void exit (int); double f (...); @@ -22,8 +21,7 @@ void h7 (volatile struct s x, ...); int main () { - if (f (1, 2.0, 3, 4.0) != 10.0) - abort (); + f (1, 2.0, 3, 4.0); g (0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); g (0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f); h1 (0, 0.0, 1, 2.0, 3, 4.0, 5, 6.0, 7, 8.0, 9); diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c index 064da121ec2..3b70fe1595a 100644 --- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c +++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c @@ -8,17 +8,20 @@ extern void abort (void); -double +void f (...) { va_list ap; va_start (ap); - double ret = va_arg (ap, int); - ret += va_arg (ap, double); - ret += va_arg (ap, int); - ret += va_arg (ap, double); + if (va_arg (ap, int) != 1) + abort(); + if (va_arg (ap, double) != 2) + abort(); + if (va_arg (ap, int) != 3) + abort(); + if (va_arg (ap, double) != 4) + abort(); va_end (ap); - return ret; } void diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c new file mode 100644 index 00000000000..23cc78d5230 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c @@ -0,0 +1,37 @@ +/* Test AAPCS layout (alignment of varargs) for callee. */ + +/* { dg-do run { target arm_eabi } } */ +/* { dg-require-effective-target arm32 } */ +/* { dg-options "-std=c23 -O2 -fno-inline" } */ + +#include <stdarg.h> + +extern void abort (void); + +typedef __attribute__((aligned (8))) int alignedint; + +void +foo (...) +{ + int i = 0; + va_list va; + va_start (va); + /* Arguments should be passed in the same registers as if they were ints. */ + for (i = 5; i >= 0; i--) + if (va_arg (va, int) != i) + abort (); + va_end (va); +} + +int +main (int argc, char **argv) +{ + alignedint a = 5; + alignedint b = 4; + alignedint c = 3; + alignedint d = 2; + alignedint e = 1; + alignedint f = 0; + foo (a, b, c, d, e, f); + return 0; +} diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c new file mode 100644 index 00000000000..c4333f33677 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c @@ -0,0 +1,31 @@ +/* Test AAPCS layout (alignment of varargs) for callee. */ + +/* { dg-do run { target arm_eabi } } */ +/* { dg-require-effective-target arm32 } */ +/* { dg-options "-std=c23 -O2 -fno-inline" } */ + +#include <stdarg.h> + +extern void abort (void); + +typedef __attribute__((aligned (8))) int alignedint; + +void +foo (...) +{ + int i = 0; + va_list va; + va_start (va); + /* alignedint should be pulled out of regs/stack just like an int. */ + for (i = 5; i >= 0; i--) + if (va_arg (va, alignedint) != i) + abort (); + va_end (va); +} + +int +main (int argc, char **argv) +{ + foo (5, 4, 3, 2, 1, 0); + return 0; +}
Ok for trunk and releases/gcc-13? Regtested on top of 945cb8490cb for arm-none-eabi, without any regression. Backporting to releases/gcc-13 will change -std=c23 to -std=c2x. -- In commit 4fe34cdcc80ac225b80670eabc38ac5e31ce8a5a, -std=c23 support was introduced to support functions without any named arguments. For arm-none-eabi, this is not as simple as placing all arguments on the stack. Align the caller to use r0, r1, r2 and r3 for arguments even for functions without any named arguments, as specified in the AAPCS. Verify that the generic test case have the arguments are in the right order and add ARM specific test cases. gcc/ChangeLog: * calls.h: Added the type of the function to function_arg_info. * calls.cc: Save the type of the function. * config/arm/arm.cc: Check in the AAPCS layout function if function has no named args. gcc/testsuite/ChangeLog: * gcc.dg/torture/c23-stdarg-split-1a.c: Detect out of order arguments. * gcc.dg/torture/c23-stdarg-split-1b.c: Likewise. * gcc.target/arm/aapcs/align_vaarg3.c: New test. * gcc.target/arm/aapcs/align_vaarg4.c: New test. Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> Co-authored-by: Yvan ROUX <yvan.roux@foss.st.com> --- gcc/calls.cc | 2 +- gcc/calls.h | 20 ++++++++-- gcc/config/arm/arm.cc | 13 ++++--- .../gcc.dg/torture/c23-stdarg-split-1a.c | 4 +- .../gcc.dg/torture/c23-stdarg-split-1b.c | 15 +++++--- .../gcc.target/arm/aapcs/align_vaarg3.c | 37 +++++++++++++++++++ .../gcc.target/arm/aapcs/align_vaarg4.c | 31 ++++++++++++++++ 7 files changed, 102 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg3.c create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/align_vaarg4.c