> Am 16.03.2024 um 08:24 schrieb Jakub Jelinek <ja...@redhat.com>:
>
> Hi!
>
> The c23-stdarg-6.c testcase I've added recently apparently works fine with
> -O0 but aborts with -O1 and higher on x86_64-linux.
> The problem is in setup of incoming varargs.
>
> Like function.cc before r14-9249 even ix86_setup_incoming_varargs assumes
> that TYPE_NO_NAMED_ARGS_STDARG_P don't have any named arguments and there
> is nothing to advance, but that is not the case for (...) functions
> returning by hidden reference which have one such artificial argument.
> If the setup_incoming_varargs hook is called from the
> if (TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (fndecl))
> && fnargs.is_empty ())
> {
> struct assign_parm_data_one data = {};
> assign_parms_setup_varargs (&all, &data, false);
> }
> spot, i.e. where there is no hidden return argument passed, arg.type
> is always NULL, while when it is called in the
> if (cfun->stdarg && !DECL_CHAIN (parm))
> assign_parms_setup_varargs (&all, &data, false);
> spot, even when it is TYPE_NO_NAMED_ARGS_STDARG_P arg.type will be non-NULL.
> The tree-stdarg.cc pass in f in c23-stdarg-6.cc at -O1 or higher determines
> that va_arg is used on integral types at most twice (loads 2 words),
> and because ix86_setup_incoming_varargs doesn't advance, the code saves
> just the %rdi and %rsi registers to the save area. But that isn't correct,
> it should save %rsi and %rdx because %rdi is the hidden return argument.
> With -O0 tree-stdarg.cc doesn't attempt to optimize and we save all the
> registers, so it works fine in that case.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?
Ok
> Now, I think we'll need the same fix also on
> aarch64, alpha, arc, csky, ia64, loongarch, mips, mmix, nios2, riscv, visium
> which have pretty much the similarly looking snippet in their hooks
> changed by the r13-3549 commit.
> Then arm, epiphany, fr30, frv, ft32, m32r, mcore, nds32, rs6000, sh
> have different changes but most likely need something similar too.
> I don't have access to most of those, could test aarch64 and rs6000 I guess.
If we have testsuite coverage after this I’d say go ahead for unmaintained
ports.
Richard
> 2024-03-16 Jakub Jelinek <ja...@redhat.com>
>
> PR target/114175
> * config/i386/i386.cc (ix86_setup_incoming_varargs): Only skip
> ix86_function_arg_advance for TYPE_NO_NAMED_ARGS_STDARG_P functions
> if arg.type is NULL.
>
> * gcc.dg/c23-stdarg-7.c: New test.
> * gcc.dg/c23-stdarg-8.c: New test.
>
> --- gcc/config/i386/i386.cc.jj 2024-03-05 10:26:19.424029862 +0100
> +++ gcc/config/i386/i386.cc 2024-03-15 23:50:08.821823213 +0100
> @@ -4643,7 +4643,8 @@ ix86_setup_incoming_varargs (cumulative_
> /* For varargs, we do not want to skip the dummy va_dcl argument.
> For stdargs, we do want to skip the last named argument. */
> next_cum = *cum;
> - if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl))
> + if ((!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl))
> + || arg.type != NULL_TREE)
> && stdarg_p (fntype))
> ix86_function_arg_advance (pack_cumulative_args (&next_cum), arg);
>
> --- gcc/testsuite/gcc.dg/c23-stdarg-7.c.jj 2024-03-15 23:50:56.857185518
> +0100
> +++ gcc/testsuite/gcc.dg/c23-stdarg-7.c 2024-03-15 23:50:45.611334813 +0100
> @@ -0,0 +1,6 @@
> +/* Test C23 variadic functions with no named parameters, or last named
> + parameter with a declaration not allowed in C17. Execution tests. */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -std=c23 -pedantic-errors" } */
> +
> +#include "c23-stdarg-4.c"
> --- gcc/testsuite/gcc.dg/c23-stdarg-8.c.jj 2024-03-15 23:51:20.814867467
> +0100
> +++ gcc/testsuite/gcc.dg/c23-stdarg-8.c 2024-03-15 23:51:06.973051224 +0100
> @@ -0,0 +1,6 @@
> +/* Test C23 variadic functions with no named parameters, or last named
> + parameter with a declaration not allowed in C17. Execution tests. */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -std=c23 -pedantic-errors" } */
> +
> +#include "c23-stdarg-6.c"
>
> Jakub
>
