Re: [Xen-devel] [PATCH] x86/asm: Use ASM_FLAG_OUT() to simplify atomic and bitop stubs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.02.17 at 15:22, <andrew.cooper3@xxxxxxxxxx> wrote:
> bitops.h cannot include asm_defns.h, because the static inlines in cpumasks.h
> result in forward declarations of the bitops.h contents.  Move ASM_FLAG_OUT()
> to a new asm/compiler.h to compensate.
> 
> While making changes, switch bool_t to bool and use named asm parameters.
> 
> No (intended) functional change.  Without GCC flags, the disassembly is
> identical before and after this patch.  With GCC flags however, this patch
> seems to cause GCC to make instruction scheduling decisions.
> 
> The first place with significant changes in the disassembly is the the middle
> of do_domctl(), which switches from:
> 
>     lock decl 0x1d8(%rax)
>     sete   %r14b
>     mov    $0xffffffffffffffea,%rbx
>     test   %r14b,%r14b
>     je     ffff82d0801034d4
> 
> to:
> 
>     lock decl 0x1d8(%rax)
>     mov    $0x0,%r14d
>     mov    $0xffffffffffffffea,%rbx
>     jne    ffff82d0801034d4
> 
> avoiding the use of %r14d as an intermediate for calculating the conditional
> jump, freeing it up for later use.  As a result, the compiled code is a little
> more efficient and smaller overall.

I have to admit that I find this rather surprising, especially since
(leaving the naming of the asm() operands aside) the preprocessed
source should not change afaict. It being unexpected I think calls
for better understanding what's going on here.

> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -107,6 +107,7 @@ extern unsigned char boot_edid_info[128];
>  #define asmlinkage
>  
>  #include <xen/const.h>
> +#include <asm/compiler.h>

Why here? You need it in bitops.h and maybe (it's not really used
there) asm_defns.h only for now.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel