Re: [PATCH] calls: Fix up TYPE_NO_NAMED_ARGS_STDARG_P handling [PR107453]

[Jakub Jelinek]

On Tue, Feb 27, 2024 at 04:41:32PM +0000, Richard Earnshaw wrote:
> > 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?

I'm afraid I haven't heard of that target hook before.
All I was doing with that change was fixing a regression reported in the PR
for ppc64le/sparc/nvptx/loongarch at least.

The TYPE_NO_NAMED_ARGS_STDARG_P functions (C23 fns like void foo (...) {})
have NULL type_arg_types, so the list_length (type_arg_types) isn't done for
it, but it should be handled as if it was non-NULL but list length was 0.

So, for the
  if (type_arg_types != 0)
    n_named_args
      = (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;
case, I think guarding it by any target hooks is wrong, although
I guess it should have been
    n_named_args = structure_value_addr_parm;
instead of
    n_named_args = 0;

For the second
  if (type_arg_types != 0
      && targetm.calls.strict_argument_naming (args_so_far))
    ;
  else if (type_arg_types != 0
           && ! 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;
bet (but no testing done, don't even know which targets return what for
those hooks) we should treat those as if type_arg_types was non-NULL
with 0 elements in the list, except the --n_named_args doesn't make sense
because that would decrease it to -1.
So perhaps
  if ((type_arg_types != 0 || TYPE_NO_NAMED_ARGS_STDARG_P (funtype))
      && targetm.calls.strict_argument_naming (args_so_far))
    ;
  else if (type_arg_types != 0
           && ! 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)
           && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far)))
    ;
  else
    /* Treat all args as named.  */
    n_named_args = num_actuals;

(or n_named_args = 0; instead of ; before the final else?  Dunno).
I guess we need some testsuite coverage for caller/callee ABI match of
struct S { char p[64]; };
struct S foo (...);

        Jakub