On 19.11.2024 16:31, Andrew Cooper wrote:
> On 19/11/2024 10:34 am, Roger Pau Monne wrote:
> Overall this is far more legible, and I'm tempted to take it on that
> justification alone. But this is Jan's pile of magic.
>
> There is a weird effect from this change:
>
> add/remove: 2/0 grow/shrink: 2/2 up/down: 740/-739 (1)
> Function old new delta
> build_symbol_table - 686 +686
> build_symbol_table.cold - 48 +48
> pv_map_ldt_shadow_page 641 644 +3
> pv_emulate_gate_op 2919 2922 +3
> livepatch_op.cold 557 509 -48
> livepatch_op 5952 5261 -691
>
> which is clearly changing inlining decisions. I suspect it's related to...
>
>> --- a/xen/arch/x86/usercopy.c
>> +++ b/xen/arch/x86/usercopy.c
>> @@ -11,23 +11,23 @@
>> #include <asm/uaccess.h>
>>
>> #ifndef GUARD
>> -# define GUARD UA_KEEP
>> +# define GUARD 1
>> #endif
>>
>> unsigned int copy_to_guest_ll(void __user *to, const void *from, unsigned
>> int n)
>> {
>> - GUARD(unsigned dummy);
>> + unsigned __maybe_unused dummy;
>
> ... this. This doesn't need to be __maybe_unused, because ...
>
>>
>> stac();
>> asm volatile (
>> - GUARD(
>> + ".if " STR(GUARD) "\n"
>> " guest_access_mask_ptr %[to], %q[scratch1], %q[scratch2]\n"
>> - )
>> + ".endif\n"
>> "1: rep movsb\n"
>> "2:\n"
>> _ASM_EXTABLE(1b, 2b)
>> - : [cnt] "+c" (n), [to] "+D" (to), [from] "+S" (from)
>> - GUARD(, [scratch1] "=&r" (dummy), [scratch2] "=&r" (dummy))
>> + : [cnt] "+c" (n), [to] "+D" (to), [from] "+S" (from),
>> + [scratch1] "=&r" (dummy), [scratch2] "=&r" (dummy)
>
> ... these parameters are referenced unconditionally.
>
> However, it does mean the compiler is spilling the scratch registers
> even when guard is 0. I expect this is what is affecting the inlining
> decisions.
Or the increased number of newlines that the asm() expands to (or the
combination of both).
Jan
