[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>

I have confirmed using:

diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index ef7e70b..de79443 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -197,7 +197,7 @@ static inline int atomic_dec_and_test(atomic_t *v)

 #ifdef __GCC_ASM_FLAG_OUTPUTS__
     asm volatile ( "lock; decl %0"
-                   : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
+                   : "+m" (*(volatile int *)&v->counter), "=@ccnz" (c)
                    :: "memory" );
 #else
     asm volatile ( "lock; decl %0; setz %1"

that compiler explicitly chose to insert the sete instruction despite the
"=@ccz" condition.  The resulting hypervisor does work perfectly well, so I
guess this will be down to some internal details of GCC's code generation.
---
 xen/include/asm-x86/asm_defns.h |  6 ---
 xen/include/asm-x86/atomic.h    | 64 ++++++++++++-------------------
 xen/include/asm-x86/bitops.h    | 85 ++++++++++++++---------------------------
 xen/include/asm-x86/compiler.h  | 20 ++++++++++
 xen/include/asm-x86/config.h    |  1 +
 5 files changed, 73 insertions(+), 103 deletions(-)
 create mode 100644 xen/include/asm-x86/compiler.h

diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index 220ae2e..f1c6fa1 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -413,10 +413,4 @@ static always_inline void stac(void)
 #define REX64_PREFIX "rex64/"
 #endif
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-# define ASM_FLAG_OUT(yes, no) yes
-#else
-# define ASM_FLAG_OUT(yes, no) no
-#endif
-
 #endif /* __X86_ASM_DEFNS_H__ */
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index ef7e70b..d2a311c 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -133,17 +133,13 @@ static inline int atomic_sub_return(int i, atomic_t *v)
 
 static inline int atomic_sub_and_test(int i, atomic_t *v)
 {
-    bool_t c;
-
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; subl %2,%0"
-                   : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
-                   : "ir" (i) : "memory" );
-#else
-    asm volatile ( "lock; subl %2,%0; setz %1"
-                   : "+m" (*(volatile int *)&v->counter), "=qm" (c)
-                   : "ir" (i) : "memory" );
-#endif
+    bool c;
+
+    asm volatile ( "lock; subl %[i], %[counter]\n\t"
+                   ASM_FLAG_OUT(, "setz %[zf]\n\t")
+                   : [counter] "+m" (*(volatile int *)&v->counter),
+                     [zf] ASM_FLAG_OUT("=@ccz", "=qm") (c)
+                   : [i] "ir" (i) : "memory" );
 
     return c;
 }
@@ -163,17 +159,13 @@ static inline int atomic_inc_return(atomic_t *v)
 
 static inline int atomic_inc_and_test(atomic_t *v)
 {
-    bool_t c;
+    bool c;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; incl %0"
-                   : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
-                   :: "memory" );
-#else
-    asm volatile ( "lock; incl %0; setz %1"
-                   : "+m" (*(volatile int *)&v->counter), "=qm" (c)
+    asm volatile ( "lock; incl %[counter]\n\t"
+                   ASM_FLAG_OUT(, "setz %[zf]\n\t")
+                   : [counter] "+m" (*(volatile int *)&v->counter),
+                     [zf] ASM_FLAG_OUT("=@ccz", "=qm") (c)
                    :: "memory" );
-#endif
 
     return c;
 }
@@ -193,34 +185,26 @@ static inline int atomic_dec_return(atomic_t *v)
 
 static inline int atomic_dec_and_test(atomic_t *v)
 {
-    bool_t c;
+    bool c;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; decl %0"
-                   : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
+    asm volatile ( "lock; decl %[counter]\n\t"
+                   ASM_FLAG_OUT(, "setz %[zf]\n\t")
+                   : [counter] "+m" (*(volatile int *)&v->counter),
+                     [zf] ASM_FLAG_OUT("=@ccz", "=qm") (c)
                    :: "memory" );
-#else
-    asm volatile ( "lock; decl %0; setz %1"
-                   : "+m" (*(volatile int *)&v->counter), "=qm" (c)
-                   :: "memory" );
-#endif
 
     return c;
 }
 
 static inline int atomic_add_negative(int i, atomic_t *v)
 {
-    bool_t c;
-
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; addl %2,%0"
-                   : "+m" (*(volatile int *)&v->counter), "=@ccs" (c)
-                   : "ir" (i) : "memory" );
-#else
-    asm volatile ( "lock; addl %2,%0; sets %1"
-                   : "+m" (*(volatile int *)&v->counter), "=qm" (c)
-                   : "ir" (i) : "memory" );
-#endif
+    bool c;
+
+    asm volatile ( "lock; addl %[i], %[counter]\n\t"
+                   ASM_FLAG_OUT(, "sets %[sf]\n\t")
+                   : [counter] "+m" (*(volatile int *)&v->counter),
+                     [sf] ASM_FLAG_OUT("=@ccs", "=qm") (c)
+                   : [i] "ir" (i) : "memory" );
 
     return c;
 }
diff --git a/xen/include/asm-x86/bitops.h b/xen/include/asm-x86/bitops.h
index 0f18645..440abb7 100644
--- a/xen/include/asm-x86/bitops.h
+++ b/xen/include/asm-x86/bitops.h
@@ -144,13 +144,10 @@ static inline int test_and_set_bit(int nr, volatile void 
*addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; btsl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "lock; btsl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "lock; btsl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -172,15 +169,10 @@ static inline int __test_and_set_bit(int nr, void *addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "btsl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "btsl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "btsl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -201,13 +193,10 @@ static inline int test_and_clear_bit(int nr, volatile 
void *addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; btrl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "lock; btrl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "lock; btrl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -229,15 +218,10 @@ static inline int __test_and_clear_bit(int nr, void *addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "btrl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "btrl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "btrl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -251,15 +235,10 @@ static inline int __test_and_change_bit(int nr, void 
*addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "btcl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "btcl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (*(int *)addr)
-                   : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "btcl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -280,13 +259,10 @@ static inline int test_and_change_bit(int nr, volatile 
void *addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "lock; btcl %2,%1"
-                   : "=@ccc" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "lock; btcl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "lock; btcl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -305,15 +281,10 @@ static inline int variable_test_bit(int nr, const 
volatile void *addr)
 {
     int oldbit;
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-    asm volatile ( "btl %2,%1"
-                   : "=@ccc" (oldbit)
-                   : "m" (CONST_ADDR), "Ir" (nr) : "memory" );
-#else
-    asm volatile ( "btl %2,%1\n\tsbbl %0,%0"
-                   : "=r" (oldbit)
-                   : "m" (CONST_ADDR), "Ir" (nr) : "memory" );
-#endif
+    asm volatile ( "btl %[nr], %[addr]\n\t"
+                   ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
+                   : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit)
+                   : [addr] "m" (CONST_ADDR), [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
diff --git a/xen/include/asm-x86/compiler.h b/xen/include/asm-x86/compiler.h
new file mode 100644
index 0000000..1fd6a8d
--- /dev/null
+++ b/xen/include/asm-x86/compiler.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_X86_COMPILER_H__
+#define __ASM_X86_COMPILER_H__
+
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+# define ASM_FLAG_OUT(yes, no) yes
+#else
+# define ASM_FLAG_OUT(yes, no) no
+#endif
+
+#endif /* __ASM_X86_COMPILER_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 6fd84e7..b48a0de 100644
--- 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>
 
 #define PML4_ENTRY_BITS  39
 #define PML4_ENTRY_BYTES (_AC(1,UL) << PML4_ENTRY_BITS)
-- 
2.1.4


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.