Re: [PATCH 2/5] support: don't pass to resolv_test_start a big struct by value

[Florian Weimer <fw at deneb dot enyo dot de>]

* Konstantin Kharlamov:

> On Пн, Mar 25, 2019 at 13:35:56, Florian Weimer <fw@deneb.enyo.de> 
> wrote:
>> * Konstantin Kharlamov:
>> 
>>>  On Пн, Mar 25, 2019 at 11:33:28, Florian Weimer <fw@deneb.enyo.de>
>>>  wrote:
>>>>  * Konstantin Kharlamov:
>>>> 
>>>>>   diff --git a/support/resolv_test.h b/support/resolv_test.h
>>>>>   index c9e48205ab..880330ad5c 100644
>>>>>   --- a/support/resolv_test.h
>>>>>   +++ b/support/resolv_test.h
>>>>>   @@ -116,7 +116,7 @@ void resolv_test_init (void);
>>>>>       needed.  As a side effect, NSS is reconfigured to use nss_dns
>>>>>  only
>>>>>       for aplicable databases, and the process may enter a network
>>>>>       namespace for better isolation.  */
>>>>>   -struct resolv_test *resolv_test_start (struct
>>>>>  resolv_redirect_config);
>>>>>   +struct resolv_test *resolv_test_start (const struct
>>>>>  resolv_redirect_config*);
>>>> 
>>>>  This patch would adjusting all the tests that call 
>>>> resolv_test_start,
>>>>  and these changes are missing from the patch.  It is unclear how 
>>>> this
>>>>  change would be an improvement because most tests call
>>>>  resolv_test_start exactly once and the parameter object is never 
>>>> used
>>>>  again.  Passing a pointer requires writing the argument object to 
>>>> the
>>>>  stack *and* supplying its address to resolv_test_start, which 
>>>> requires
>>>>  more work.  (Maybe some targets have more optimized code for struct
>>>>  initialization than passing many zero arguments, but that's a GCC
>>>>  issue which will eventually be fixed.)
>>> 
>>>  Wow, are you saying that in the assembly the address that gets 
>>> passed
>>>  to resolve_test_start() would be not the address of struct, but 
>>> instead
>>>  an address of an addres on a stack that is an address of the 
>>> struct…?
>> 
>> With the pointer-to-struct argument, you need to write the temporary
>> object to the stack and pass its address.
>> 
>> With the struct argument, you need to write the temporary object to
>> the stack, in the form of an argument list.
>> 
>> The second case avoids computing and passing the address of the
>> temporary object.
>
> Can't reproduce here. Testcase:

You need to look at this:

struct data {
  long a0, a1, a2, a3, a4, a5, a6, a7, a8, a9;
};

void object_parameter (struct data);
void pointer_parameter (const struct data *);

void
call_object_parameter (void)
{
  object_parameter ((struct data) { 0 });
}

void
call_pointer_parameter (void)
{
  struct data obj = { 0 };
  pointer_parameter (&obj);
}

This produces:

call_object_parameter:
	subq	$88, %rsp
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	pushq	$0
	call	object_parameter@PLT
	addq	$168, %rsp
	ret

call_pointer_parameter:
	subq	$88, %rsp
	pxor	%xmm0, %xmm0
	movq	%rsp, %rdi
	movaps	%xmm0, (%rsp)
	movaps	%xmm0, 16(%rsp)
	movaps	%xmm0, 32(%rsp)
	movaps	%xmm0, 48(%rsp)
	movaps	%xmm0, 64(%rsp)
	call	pointer_parameter@PLT
	addq	$88, %rsp
	ret

The additional instruction is:

	movq	%rsp, %rdi

This is *required* by the ABI.  The rest of the differences are due to
inefficiencies in the i386 backend and could be fixed there.  But the
movq *cannot* be optimized out for the pointer-passing case.