On 09/01/2023 10:32, Jakub Jelinek via Gcc-patches wrote:
> Hi!
> 
> On powerpc64le-linux, the following patch fixes
> -FAIL: gcc.dg/c2x-stdarg-4.c execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O0  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O1  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O2  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O2 -flto 
> -fno-use-linker-plugin -flto-partition=none  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O2 -flto -fuse-linker-plugin 
> -fno-fat-lto-objects  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -O3 -g  execution test
> -FAIL: gcc.dg/torture/c2x-stdarg-split-1a.c   -Os  execution test
> The problem is mismatch between the caller and callee side.
> On the callee side, we do:
>   /* NAMED_ARG is a misnomer.  We really mean 'non-variadic'. */
>   if (!cfun->stdarg)
>     data->arg.named = 1;  /* No variadic parms.  */
>   else if (DECL_CHAIN (parm))
>     data->arg.named = 1;  /* Not the last non-variadic parm. */
>   else if (targetm.calls.strict_argument_naming (all->args_so_far))
>     data->arg.named = 1;  /* Only variadic ones are unnamed.  */
>   else
>     data->arg.named = 0;  /* Treat as variadic.  */
> which is later passed to the target hooks to determine if a particular
> argument is named or not.  Now, cfun->stdarg is determined from the stdarg_p
> call, which for the new C2X TYPE_NO_NAMED_ARGS_STDARG_P function types
> (rettype fn (...)) returns true.  Such functions have no named arguments,
> so data->arg.named will be 0 in function.cc.  But on the caller side,
> as TYPE_NO_NAMED_ARGS_STDARG_P function types have TYPE_ARG_TYPES NULL,
> we instead treat those calls as unprototyped even when they are prototyped
> - /* If we know nothing, treat all args as named.  */ n_named_args = 
> num_actuals;
> in 2 spots.  We need to treat the TYPE_NO_NAMED_ARGS_STDARG_P cases as
> prototyped with no named arguments.
> 
> Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux (where
> it fixes the above failures), aarch64-linux and s390x-linux, ok for trunk?
> 
> 2023-01-09  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR target/107453
>       * calls.cc (expand_call): For calls with
>       TYPE_NO_NAMED_ARGS_STDARG_P (funtype) use zero for n_named_args.
>       Formatting fix.

This one has been festering for a while; both Alexandre and Torbjorn have 
attempted to fix it recently, but I'm not sure either is really right...

On Arm this is causing all anonymous arguments to be passed on the stack, which 
is incorrect per the ABI.  On a target that uses 
'pretend_outgoing_vararg_named', why is it correct to set n_named_args to zero? 
 Is it enough to guard both the statements you've added with 
!targetm.calls.pretend_outgoing_args_named?

R.

> 
> --- gcc/calls.cc.jj   2023-01-02 09:32:28.834192105 +0100
> +++ gcc/calls.cc      2023-01-06 14:52:14.740594896 +0100
> @@ -2908,8 +2908,8 @@ expand_call (tree exp, rtx target, int i
>      }
>  
>    /* Count the arguments and set NUM_ACTUALS.  */
> -  num_actuals =
> -    call_expr_nargs (exp) + num_complex_actuals + structure_value_addr_parm;
> +  num_actuals
> +    = call_expr_nargs (exp) + num_complex_actuals + 
> structure_value_addr_parm;
>  
>    /* Compute number of named args.
>       First, do a raw count of the args for INIT_CUMULATIVE_ARGS.  */
> @@ -2919,6 +2919,8 @@ expand_call (tree exp, rtx target, int i
>        = (list_length (type_arg_types)
>        /* Count the struct value address, if it is passed as a parm.  */
>        + structure_value_addr_parm);
> +  else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype))
> +    n_named_args = 0;
>    else
>      /* If we know nothing, treat all args as named.  */
>      n_named_args = num_actuals;
> @@ -2957,6 +2959,8 @@ expand_call (tree exp, rtx target, int i
>          && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far))
>      /* Don't include the last named arg.  */
>      --n_named_args;
> +  else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype))
> +    n_named_args = 0;
>    else
>      /* Treat all args as named.  */
>      n_named_args = num_actuals;
> 
>       Jakub
> 

Reply via email to