summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2024-03-01 11:07:36 +0100
committerJakub Jelinek <jakub@redhat.com>2024-03-01 11:14:51 +0100
commitc6f5f773323ab689a665bc208c3b221db42fe624 (patch)
treefb9b578648fe92afbc470058198a9eb08f73f16a
parentd3d0fcb652748191714e4c0b2541e977a7fc7bd7 (diff)
function: Fix another TYPE_NO_NAMED_ARGS_STDARG_P spot
When looking at PR114175 (although that bug seems to be now a riscv backend bug), I've noticed that for the TYPE_NO_NAMED_ARGS_STDARG_P functions which return value through hidden reference, like #include <stdarg.h> struct S { char a[64]; }; int n; struct S foo (...) { struct S s = {}; va_list ap; va_start (ap); for (int i = 0; i < n; ++i) if ((i & 1)) s.a[0] += va_arg (ap, double); else s.a[0] += va_arg (ap, int); va_end (ap); return s; } we were incorrectly calling assign_parms_setup_varargs twice, once at the start of the function and once in if (cfun->stdarg && !DECL_CHAIN (parm)) assign_parms_setup_varargs (&all, &data, false); where parm is the last and only "named" parameter. The first call, guarded with TYPE_NO_NAMED_ARGS_STDARG_P, was added in r13-3549 and is needed for int bar (...) etc. functions using va_start/va_arg/va_end, otherwise the FOR_EACH_VEC_ELT (fnargs, i, parm) in which the other call is will not iterate at all. But we shouldn't be doing that if we have the hidden return pointer. With the following patch on the above testcase with -O0 -std=c23 the assembly difference is: pushq %rbp .cfi_def_cfa_offset 16 .cfi_offset 6, -16 movq %rsp, %rbp .cfi_def_cfa_register 6 pushq %rbx subq $192, %rsp .cfi_offset 3, -24 - movq %rdi, -192(%rbp) - movq %rsi, -184(%rbp) - movq %rdx, -176(%rbp) - movq %rcx, -168(%rbp) - movq %r8, -160(%rbp) - movq %r9, -152(%rbp) - testb %al, %al - je .L2 - movaps %xmm0, -144(%rbp) - movaps %xmm1, -128(%rbp) - movaps %xmm2, -112(%rbp) - movaps %xmm3, -96(%rbp) - movaps %xmm4, -80(%rbp) - movaps %xmm5, -64(%rbp) - movaps %xmm6, -48(%rbp) - movaps %xmm7, -32(%rbp) -.L2: movq %rdi, -312(%rbp) movq %rdi, -192(%rbp) movq %rsi, -184(%rbp) movq %rdx, -176(%rbp) movq %rcx, -168(%rbp) movq %r8, -160(%rbp) movq %r9, -152(%rbp) testb %al, %al - je .L13 + je .L12 movaps %xmm0, -144(%rbp) movaps %xmm1, -128(%rbp) movaps %xmm2, -112(%rbp) movaps %xmm3, -96(%rbp) movaps %xmm4, -80(%rbp) movaps %xmm5, -64(%rbp) movaps %xmm6, -48(%rbp) movaps %xmm7, -32(%rbp) -.L13: +.L12: plus some renumbering of labels later on which clearly shows that because of this bug, we were saving all the registers twice rather then once. With -O2 -std=c23 some of it is DCEd, but we still get subq $160, %rsp .cfi_def_cfa_offset 168 - testb %al, %al - je .L2 - movaps %xmm0, 24(%rsp) - movaps %xmm1, 40(%rsp) - movaps %xmm2, 56(%rsp) - movaps %xmm3, 72(%rsp) - movaps %xmm4, 88(%rsp) - movaps %xmm5, 104(%rsp) - movaps %xmm6, 120(%rsp) - movaps %xmm7, 136(%rsp) -.L2: movq %rdi, -24(%rsp) movq %rsi, -16(%rsp) movq %rdx, -8(%rsp) movq %rcx, (%rsp) movq %r8, 8(%rsp) movq %r9, 16(%rsp) testb %al, %al - je .L13 + je .L12 movaps %xmm0, 24(%rsp) movaps %xmm1, 40(%rsp) movaps %xmm2, 56(%rsp) movaps %xmm3, 72(%rsp) movaps %xmm4, 88(%rsp) movaps %xmm5, 104(%rsp) movaps %xmm6, 120(%rsp) movaps %xmm7, 136(%rsp) -.L13: +.L12: difference, i.e. this time not all, but the floating point args were conditionally all saved twice. 2024-03-01 Jakub Jelinek <jakub@redhat.com> * function.cc (assign_parms): Only call assign_parms_setup_varargs early for TYPE_NO_NAMED_ARGS_STDARG_P functions if fnargs is empty.
-rw-r--r--gcc/function.cc3
1 files changed, 2 insertions, 1 deletions
diff --git a/gcc/function.cc b/gcc/function.cc
index 9488181cbd9..3cef6c17bce 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -3650,7 +3650,8 @@ assign_parms (tree fndecl)
assign_parms_initialize_all (&all);
fnargs = assign_parms_augmented_arg_list (&all);
- if (TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (fndecl)))
+ 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);