[gcc r13-8396] function: Fix another TYPE_NO_NAMED_ARGS_STDARG_P spot

[gcc r13-8396] function: Fix another TYPE_NO_NAMED_ARGS_STDARG_P spot

[Jakub Jelinek]

https://gcc.gnu.org/g:5de9163a247bae57479b244d98287da73253e088

commit r13-8396-g5de9163a247bae57479b244d98287da73253e088
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 1 11:07:36 2024 +0100

    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.
    
    (cherry picked from commit c6f5f773323ab689a665bc208c3b221db42fe624)

Diff:
---
 gcc/function.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/function.cc b/gcc/function.cc
index edf0b2ec6cf..8d6c4478866 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);