Re: [PATCH] calls: Further fixes for TYPE_NO_NAMED_ARGS_STDARG_P handling [PR107453]

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 29/02/2024 15:55, Jakub Jelinek wrote:
> On Thu, Feb 29, 2024 at 02:14:05PM +0000, Richard Earnshaw wrote:
>>> I tried the above on arm, aarch64 and x86_64 and that seems fine,
>>> including the new testcase you added.
>>>
>>
>> I should mention though, that INIT_CUMULATIVE_ARGS on arm ignores
>> n_named_args entirely, it doesn't need it (I don't think it even existed
>> when the AAPCS code was added).
> 
> So far I've just checked that the new testcase passes not just on
> x86_64/i686-linux, but also on {powerpc64le,s390x,aarch64}-linux
> with vanilla trunk.
> Haven't posted this patch in patch form, plus while I'm not really sure
> whether setting n_named_args to 0 or not changing in the
> !pretend_outgoing_varargs_named is right, the setting to 0 feels more
> correct to me.  If structure_value_addr_parm is 1, the function effectively
> has a single named argument and then ... args and if the target wants
> n_named_args to be number of named arguments except the last, then that
> should be 0 rather than 1.
> 
> Thus, is the following patch ok for trunk then?

The comment at the start of the section says

  /* Now possibly adjust the number of named args.
     Normally, don't include the last named arg if anonymous args follow.
     We do include the last named arg if
     targetm.calls.strict_argument_naming() returns nonzero.
     (If no anonymous args follow, the result of list_length is actually
     one too large.  This is harmless.)

So in the case of strict_argument_naming perhaps it should return 1, but 0 for other cases.

R.

> 
> 2024-02-29  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/107453
> 	* calls.cc (expand_call): For TYPE_NO_NAMED_ARGS_STDARG_P set
> 	n_named_args initially before INIT_CUMULATIVE_ARGS to
> 	structure_value_addr_parm rather than 0, after it don't modify
> 	it if strict_argument_naming and clear only if
> 	!pretend_outgoing_varargs_named.
> 
> --- gcc/calls.cc.jj	2024-01-22 11:48:08.045847508 +0100
> +++ gcc/calls.cc	2024-02-29 16:24:47.799855912 +0100
> @@ -2938,7 +2938,7 @@ expand_call (tree exp, rtx target, int i
>  	 /* 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;
> +    n_named_args = structure_value_addr_parm;
>    else
>      /* If we know nothing, treat all args as named.  */
>      n_named_args = num_actuals;
> @@ -2970,14 +2970,15 @@ expand_call (tree exp, rtx target, int i
>       we do not have any reliable way to pass unnamed args in
>       registers, so we must force them into memory.  */
>  
> -  if (type_arg_types != 0
> +  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))
> +  else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype)
> +	   && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far))
>      n_named_args = 0;
>    else
>      /* Treat all args as named.  */
> 
> 	Jakub
>