On Mon, Feb 19, 2024 at 1:14 AM Torbjörn SVENSSON
<torbjorn.svens...@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.svens...@foss.st.com>
> Co-authored-by: Yvan ROUX <yvan.r...@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
>
