Message ID | ZfVJClv1UYu+4gaB@tucnak |
---|---|
State | New |
Headers | show |
Series | i386: Fix setup of incoming varargs for (...) functions which return large aggregates [PR114175] | expand |
> Am 16.03.2024 um 08:24 schrieb Jakub Jelinek <jakub@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 <jakub@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 >
--- 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"