Re: [PATCH] rs6000: Fix up setup_incoming_varargs [PR114175]
Hi Jakub, on 2024/3/19 01:21, Jakub Jelinek wrote: > Hi! > > The c23-stdarg-8.c test (as well as the new test below added to cover even > more cases) FAIL on powerpc64le-linux and presumably other powerpc* targets > as well. > Like in the r14-9503-g218d174961 change on x86-64 we need to advance > next_cum after the hidden return pointer argument even in case where > there are no user arguments before ... in C23. > The following patch does that. > > There is another TYPE_NO_NAMED_ARGS_STDARG_P use later on: > if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl)) > && targetm.calls.must_pass_in_stack (arg)) > first_reg_offset += rs6000_arg_size (TYPE_MODE (arg.type), arg.type); > but I believe it was added there in r13-3549-g4fe34cdc unnecessarily, > when there is no hidden return pointer argument, arg.type is NULL and > must_pass_in_stack_var_size as well as must_pass_in_stack_var_size_or_pad > return false in that case, and for the TYPE_NO_NAMED_ARGS_STDARG_P > case with hidden return pointer argument that argument should have pointer > type and it is the first argument, so must_pass_in_stack shouldn't be true > for it either. > > Bootstrapped/regtested on powerpc64le-linux, bootstrap/regtest on > powerpc64-linux running, ok for trunk? Okay for trunk (I guess all testings should go well), thanks for taking care of this! FWIW, I also tested c23-stdarg-* test cases on aix with this patch, all of them worked well. BR, Kewen > > 2024-03-18 Jakub Jelinek > > PR target/114175 > * config/rs6000/rs6000-call.cc (setup_incoming_varargs): Only skip > rs6000_function_arg_advance_1 for TYPE_NO_NAMED_ARGS_STDARG_P functions > if arg.type is NULL. > > * gcc.dg/c23-stdarg-9.c: New test. > > --- gcc/config/rs6000/rs6000-call.cc.jj 2024-01-03 12:01:19.645532834 > +0100 > +++ gcc/config/rs6000/rs6000-call.cc 2024-03-18 11:36:02.376846802 +0100 > @@ -2253,7 +2253,8 @@ setup_incoming_varargs (cumulative_args_ > >/* Skip the last named argument. */ >next_cum = *get_cumulative_args (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) > rs6000_function_arg_advance_1 (&next_cum, arg.mode, arg.type, arg.named, > 0); > > --- gcc/testsuite/gcc.dg/c23-stdarg-9.c.jj2024-03-18 11:46:17.281200214 > +0100 > +++ gcc/testsuite/gcc.dg/c23-stdarg-9.c 2024-03-18 11:46:26.826065998 > +0100 > @@ -0,0 +1,284 @@ > +/* 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 > + > +struct S { int a[1024]; }; > + > +int > +f1 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f2 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f3 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f4 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f5 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f6 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f7 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +int > +f8 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + r += va_arg (ap, int); > + va_end (ap); > + return r; > +} > + > +struct S > +s1 (...) > +{ > + int r = 0; > + va_list ap; > + va_start (ap); > + r += va_arg (ap, int); > + va_end (ap); > + struct S s = {};
[PATCH] rs6000: Fix up setup_incoming_varargs [PR114175]
Hi! The c23-stdarg-8.c test (as well as the new test below added to cover even more cases) FAIL on powerpc64le-linux and presumably other powerpc* targets as well. Like in the r14-9503-g218d174961 change on x86-64 we need to advance next_cum after the hidden return pointer argument even in case where there are no user arguments before ... in C23. The following patch does that. There is another TYPE_NO_NAMED_ARGS_STDARG_P use later on: if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl)) && targetm.calls.must_pass_in_stack (arg)) first_reg_offset += rs6000_arg_size (TYPE_MODE (arg.type), arg.type); but I believe it was added there in r13-3549-g4fe34cdc unnecessarily, when there is no hidden return pointer argument, arg.type is NULL and must_pass_in_stack_var_size as well as must_pass_in_stack_var_size_or_pad return false in that case, and for the TYPE_NO_NAMED_ARGS_STDARG_P case with hidden return pointer argument that argument should have pointer type and it is the first argument, so must_pass_in_stack shouldn't be true for it either. Bootstrapped/regtested on powerpc64le-linux, bootstrap/regtest on powerpc64-linux running, ok for trunk? 2024-03-18 Jakub Jelinek PR target/114175 * config/rs6000/rs6000-call.cc (setup_incoming_varargs): Only skip rs6000_function_arg_advance_1 for TYPE_NO_NAMED_ARGS_STDARG_P functions if arg.type is NULL. * gcc.dg/c23-stdarg-9.c: New test. --- gcc/config/rs6000/rs6000-call.cc.jj 2024-01-03 12:01:19.645532834 +0100 +++ gcc/config/rs6000/rs6000-call.cc2024-03-18 11:36:02.376846802 +0100 @@ -2253,7 +2253,8 @@ setup_incoming_varargs (cumulative_args_ /* Skip the last named argument. */ next_cum = *get_cumulative_args (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) rs6000_function_arg_advance_1 (&next_cum, arg.mode, arg.type, arg.named, 0); --- gcc/testsuite/gcc.dg/c23-stdarg-9.c.jj 2024-03-18 11:46:17.281200214 +0100 +++ gcc/testsuite/gcc.dg/c23-stdarg-9.c 2024-03-18 11:46:26.826065998 +0100 @@ -0,0 +1,284 @@ +/* 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 + +struct S { int a[1024]; }; + +int +f1 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f2 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f3 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f4 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f5 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f6 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f7 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +int +f8 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + return r; +} + +struct S +s1 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + va_end (ap); + struct S s = {}; + s.a[0] = r; + return s; +} + +struct S +s2 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + struct S s = {}; + s.a[0] = r; + return s; +} + +struct S +s3 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + va_end (ap); + struct S s = {}; + s.a[0] = r; + return s; +} + +struct S +s4 (...) +{ + int r = 0; + va_list ap; + va_start (ap); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int); + r += va_arg (ap, int);