* [PATCH] x86: use gcc6'es flags asm() output support
@ 2016-07-01 15:01 Jan Beulich
2016-07-01 15:38 ` Andrew Cooper
2016-07-04 6:24 ` Tian, Kevin
0 siblings, 2 replies; 12+ messages in thread
From: Jan Beulich @ 2016-07-01 15:01 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 10861 bytes --]
..., rendering affected code more efficient and smaller.
Note that in atomic.h this at once does away with the redundant output
and input specifications of the memory location touched.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Do we want to abstract the pattern
asm ( "...; set<cc> %<out>" : "=<cons>" (var) ... )
matching
asm ( "..." : "=@cc<cc>" (var) ... )
via some macro? While this would eliminate many (all?) of the
conditionals added here, it would result in the : no longer being
visible in the actual source, making the asm()s look somewhat odd.
Otherwise, to limit code duplication, it may be preferable to put
the #ifdef-s inside the asm()s instead of around them.
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -610,7 +610,12 @@ do {
*/
static bool_t even_parity(uint8_t v)
{
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "test %1,%1" : "=@ccp" (v) : "q" (v) );
+#else
asm ( "test %1,%1; setp %0" : "=qm" (v) : "q" (v) );
+#endif
+
return v;
}
@@ -832,8 +837,14 @@ static int read_ulong(
static bool_t mul_dbl(unsigned long m[2])
{
bool_t rc;
+
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "mul %1" : "+a" (m[0]), "+d" (m[1]), "=@cco" (rc) );
+#else
asm ( "mul %1; seto %2"
: "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
+#endif
+
return rc;
}
@@ -845,8 +856,14 @@ static bool_t mul_dbl(unsigned long m[2]
static bool_t imul_dbl(unsigned long m[2])
{
bool_t rc;
+
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "imul %1" : "+a" (m[0]), "+d" (m[1]), "=@cco" (rc) );
+#else
asm ( "imul %1; seto %2"
: "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
+#endif
+
return rc;
}
@@ -4651,9 +4668,15 @@ x86_emulate(
case 0xbc: /* bsf or tzcnt */ {
bool_t zf;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "bsf %2,%0"
+ : "=r" (dst.val), "=@ccz" (zf)
+ : "rm" (src.val) );
+#else
asm ( "bsf %2,%0; setz %1"
: "=r" (dst.val), "=qm" (zf)
: "rm" (src.val) );
+#endif
_regs.eflags &= ~EFLG_ZF;
if ( (vex.pfx == vex_f3) && vcpu_has_bmi1() )
{
@@ -4677,9 +4700,15 @@ x86_emulate(
case 0xbd: /* bsr or lzcnt */ {
bool_t zf;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "bsr %2,%0"
+ : "=r" (dst.val), "=@ccz" (zf)
+ : "rm" (src.val) );
+#else
asm ( "bsr %2,%0; setz %1"
: "=r" (dst.val), "=qm" (zf)
: "rm" (src.val) );
+#endif
_regs.eflags &= ~EFLG_ZF;
if ( (vex.pfx == vex_f3) && vcpu_has_lzcnt() )
{
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -193,12 +193,18 @@ static inline void atomic_sub(int i, ato
*/
static inline int atomic_sub_and_test(int i, atomic_t *v)
{
- unsigned char c;
+ 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
- asm volatile (
- "lock; subl %2,%0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "ir" (i), "m" (*(volatile int *)&v->counter) : "memory" );
return c;
}
@@ -240,13 +246,19 @@ static inline void atomic_dec(atomic_t *
*/
static inline int atomic_dec_and_test(atomic_t *v)
{
- unsigned char c;
+ bool_t c;
- asm volatile (
- "lock; decl %0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "m" (*(volatile int *)&v->counter) : "memory" );
- return c != 0;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm volatile ( "lock; decl %0"
+ : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
+ :: "memory" );
+#else
+ asm volatile ( "lock; decl %0; setz %1"
+ : "+m" (*(volatile int *)&v->counter), "=qm" (c)
+ :: "memory" );
+#endif
+
+ return c;
}
/**
@@ -259,13 +271,19 @@ static inline int atomic_dec_and_test(at
*/
static inline int atomic_inc_and_test(atomic_t *v)
{
- unsigned char c;
+ bool_t c;
- asm volatile (
- "lock; incl %0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "m" (*(volatile int *)&v->counter) : "memory" );
- return c != 0;
+#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)
+ :: "memory" );
+#endif
+
+ return c;
}
/**
@@ -279,12 +297,18 @@ static inline int atomic_inc_and_test(at
*/
static inline int atomic_add_negative(int i, atomic_t *v)
{
- unsigned char c;
+ 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
- asm volatile (
- "lock; addl %2,%0; sets %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "ir" (i), "m" (*(volatile int *)&v->counter) : "memory" );
return c;
}
--- a/xen/include/asm-x86/bitops.h
+++ b/xen/include/asm-x86/bitops.h
@@ -145,8 +145,14 @@ static inline int test_and_set_bit(int n
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_set_bit(nr, addr) ({ \
@@ -167,10 +173,16 @@ static inline int __test_and_set_bit(int
{
int oldbit;
- asm volatile (
- "btsl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_set_bit(nr, addr) ({ \
@@ -190,8 +202,14 @@ static inline int test_and_clear_bit(int
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_clear_bit(nr, addr) ({ \
@@ -212,10 +230,16 @@ static inline int __test_and_clear_bit(i
{
int oldbit;
- asm volatile (
- "btrl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_clear_bit(nr, addr) ({ \
@@ -228,10 +252,16 @@ static inline int __test_and_change_bit(
{
int oldbit;
- asm volatile (
- "btcl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_change_bit(nr, addr) ({ \
@@ -251,8 +281,14 @@ static inline int test_and_change_bit(in
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_change_bit(nr, addr) ({ \
@@ -270,10 +306,16 @@ static inline int variable_test_bit(int
{
int oldbit;
- asm volatile (
- "btl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit)
- : "m" (CONST_ADDR), "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -406,12 +406,17 @@ static inline bool_t __vmread_safe(unsig
VMREAD_OPCODE MODRM_EAX_ECX
#endif
/* CF==1 or ZF==1 --> rc = 0 */
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ : "=@ccnbe" (okay),
+#else
"setnbe %0"
+ : "=qm" (okay),
+#endif
#ifdef HAVE_GAS_VMX
- : "=qm" (okay), "=rm" (*value)
+ "=rm" (*value)
: "r" (field));
#else
- : "=qm" (okay), "=c" (*value)
+ "=c" (*value)
: "a" (field));
#endif
[-- Attachment #2: x86-gcc6-flags-output.patch --]
[-- Type: text/plain, Size: 10904 bytes --]
x86: use gcc6'es flags asm() output support
..., rendering affected code more efficient and smaller.
Note that in atomic.h this at once does away with the redundant output
and input specifications of the memory location touched.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Do we want to abstract the pattern
asm ( "...; set<cc> %<out>" : "=<cons>" (var) ... )
matching
asm ( "..." : "=@cc<cc>" (var) ... )
via some macro? While this would eliminate many (all?) of the
conditionals added here, it would result in the : no longer being
visible in the actual source, making the asm()s look somewhat odd.
Otherwise, to limit code duplication, it may be preferable to put
the #ifdef-s inside the asm()s instead of around them.
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -610,7 +610,12 @@ do {
*/
static bool_t even_parity(uint8_t v)
{
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "test %1,%1" : "=@ccp" (v) : "q" (v) );
+#else
asm ( "test %1,%1; setp %0" : "=qm" (v) : "q" (v) );
+#endif
+
return v;
}
@@ -832,8 +837,14 @@ static int read_ulong(
static bool_t mul_dbl(unsigned long m[2])
{
bool_t rc;
+
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "mul %1" : "+a" (m[0]), "+d" (m[1]), "=@cco" (rc) );
+#else
asm ( "mul %1; seto %2"
: "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
+#endif
+
return rc;
}
@@ -845,8 +856,14 @@ static bool_t mul_dbl(unsigned long m[2]
static bool_t imul_dbl(unsigned long m[2])
{
bool_t rc;
+
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "imul %1" : "+a" (m[0]), "+d" (m[1]), "=@cco" (rc) );
+#else
asm ( "imul %1; seto %2"
: "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
+#endif
+
return rc;
}
@@ -4651,9 +4668,15 @@ x86_emulate(
case 0xbc: /* bsf or tzcnt */ {
bool_t zf;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "bsf %2,%0"
+ : "=r" (dst.val), "=@ccz" (zf)
+ : "rm" (src.val) );
+#else
asm ( "bsf %2,%0; setz %1"
: "=r" (dst.val), "=qm" (zf)
: "rm" (src.val) );
+#endif
_regs.eflags &= ~EFLG_ZF;
if ( (vex.pfx == vex_f3) && vcpu_has_bmi1() )
{
@@ -4677,9 +4700,15 @@ x86_emulate(
case 0xbd: /* bsr or lzcnt */ {
bool_t zf;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm ( "bsr %2,%0"
+ : "=r" (dst.val), "=@ccz" (zf)
+ : "rm" (src.val) );
+#else
asm ( "bsr %2,%0; setz %1"
: "=r" (dst.val), "=qm" (zf)
: "rm" (src.val) );
+#endif
_regs.eflags &= ~EFLG_ZF;
if ( (vex.pfx == vex_f3) && vcpu_has_lzcnt() )
{
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -193,12 +193,18 @@ static inline void atomic_sub(int i, ato
*/
static inline int atomic_sub_and_test(int i, atomic_t *v)
{
- unsigned char c;
+ 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
- asm volatile (
- "lock; subl %2,%0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "ir" (i), "m" (*(volatile int *)&v->counter) : "memory" );
return c;
}
@@ -240,13 +246,19 @@ static inline void atomic_dec(atomic_t *
*/
static inline int atomic_dec_and_test(atomic_t *v)
{
- unsigned char c;
+ bool_t c;
- asm volatile (
- "lock; decl %0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "m" (*(volatile int *)&v->counter) : "memory" );
- return c != 0;
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ asm volatile ( "lock; decl %0"
+ : "+m" (*(volatile int *)&v->counter), "=@ccz" (c)
+ :: "memory" );
+#else
+ asm volatile ( "lock; decl %0; setz %1"
+ : "+m" (*(volatile int *)&v->counter), "=qm" (c)
+ :: "memory" );
+#endif
+
+ return c;
}
/**
@@ -259,13 +271,19 @@ static inline int atomic_dec_and_test(at
*/
static inline int atomic_inc_and_test(atomic_t *v)
{
- unsigned char c;
+ bool_t c;
- asm volatile (
- "lock; incl %0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "m" (*(volatile int *)&v->counter) : "memory" );
- return c != 0;
+#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)
+ :: "memory" );
+#endif
+
+ return c;
}
/**
@@ -279,12 +297,18 @@ static inline int atomic_inc_and_test(at
*/
static inline int atomic_add_negative(int i, atomic_t *v)
{
- unsigned char c;
+ 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
- asm volatile (
- "lock; addl %2,%0; sets %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "ir" (i), "m" (*(volatile int *)&v->counter) : "memory" );
return c;
}
--- a/xen/include/asm-x86/bitops.h
+++ b/xen/include/asm-x86/bitops.h
@@ -145,8 +145,14 @@ static inline int test_and_set_bit(int n
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_set_bit(nr, addr) ({ \
@@ -167,10 +173,16 @@ static inline int __test_and_set_bit(int
{
int oldbit;
- asm volatile (
- "btsl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_set_bit(nr, addr) ({ \
@@ -190,8 +202,14 @@ static inline int test_and_clear_bit(int
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_clear_bit(nr, addr) ({ \
@@ -212,10 +230,16 @@ static inline int __test_and_clear_bit(i
{
int oldbit;
- asm volatile (
- "btrl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_clear_bit(nr, addr) ({ \
@@ -228,10 +252,16 @@ static inline int __test_and_change_bit(
{
int oldbit;
- asm volatile (
- "btcl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit), "+m" (*(int *)addr)
- : "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
#define __test_and_change_bit(nr, addr) ({ \
@@ -251,8 +281,14 @@ static inline int test_and_change_bit(in
{
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");
+ : "=r" (oldbit), "+m" (ADDR) : "Ir" (nr) : "memory" );
+#endif
+
return oldbit;
}
#define test_and_change_bit(nr, addr) ({ \
@@ -270,10 +306,16 @@ static inline int variable_test_bit(int
{
int oldbit;
- asm volatile (
- "btl %2,%1\n\tsbbl %0,%0"
- : "=r" (oldbit)
- : "m" (CONST_ADDR), "Ir" (nr) : "memory" );
+#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
+
return oldbit;
}
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -406,12 +406,17 @@ static inline bool_t __vmread_safe(unsig
VMREAD_OPCODE MODRM_EAX_ECX
#endif
/* CF==1 or ZF==1 --> rc = 0 */
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ : "=@ccnbe" (okay),
+#else
"setnbe %0"
+ : "=qm" (okay),
+#endif
#ifdef HAVE_GAS_VMX
- : "=qm" (okay), "=rm" (*value)
+ "=rm" (*value)
: "r" (field));
#else
- : "=qm" (okay), "=c" (*value)
+ "=c" (*value)
: "a" (field));
#endif
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 15:01 [PATCH] x86: use gcc6'es flags asm() output support Jan Beulich
@ 2016-07-01 15:38 ` Andrew Cooper
2016-07-01 15:54 ` Jan Beulich
2016-07-01 16:10 ` Jan Beulich
2016-07-04 6:24 ` Tian, Kevin
1 sibling, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-07-01 15:38 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Jun Nakajima
On 01/07/16 16:01, Jan Beulich wrote:
> ..., rendering affected code more efficient and smaller.
Ooh - this is a neat feature.
>
> Note that in atomic.h this at once does away with the redundant output
> and input specifications of the memory location touched.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Do we want to abstract the pattern
> asm ( "...; set<cc> %<out>" : "=<cons>" (var) ... )
> matching
> asm ( "..." : "=@cc<cc>" (var) ... )
> via some macro? While this would eliminate many (all?) of the
> conditionals added here, it would result in the : no longer being
> visible in the actual source, making the asm()s look somewhat odd.
> Otherwise, to limit code duplication, it may be preferable to put
> the #ifdef-s inside the asm()s instead of around them.
I would recommend not abstracting them. asm() statements are already
subtle/hard enough to get right, and hiding the :'s will only make
matters worse.
As for interleaving inside the asm statement itself, we already have
precedent for that with the HAVE_GAS_* predicates. It would make the
patch rather larger, but might end up looking cleaner. It is probably
also worth switching to named parameters to reduce the risk of getting
positional parameters out of order.
As for the content of the change itself, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 15:38 ` Andrew Cooper
@ 2016-07-01 15:54 ` Jan Beulich
2016-07-01 16:10 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-07-01 15:54 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 01.07.16 at 17:38, <andrew.cooper3@citrix.com> wrote:
> On 01/07/16 16:01, Jan Beulich wrote:
>> TBD: Do we want to abstract the pattern
>> asm ( "...; set<cc> %<out>" : "=<cons>" (var) ... )
>> matching
>> asm ( "..." : "=@cc<cc>" (var) ... )
>> via some macro? While this would eliminate many (all?) of the
>> conditionals added here, it would result in the : no longer being
>> visible in the actual source, making the asm()s look somewhat odd.
>> Otherwise, to limit code duplication, it may be preferable to put
>> the #ifdef-s inside the asm()s instead of around them.
>
> I would recommend not abstracting them. asm() statements are already
> subtle/hard enough to get right, and hiding the :'s will only make
> matters worse.
Right, that's why I didn't do that right away. Otoh the alternative
asm macros hide colons too.
> As for interleaving inside the asm statement itself, we already have
> precedent for that with the HAVE_GAS_* predicates. It would make the
> patch rather larger, but might end up looking cleaner. It is probably
> also worth switching to named parameters to reduce the risk of getting
> positional parameters out of order.
I don't really see that risk, but I'll try to judge on a case by case
basis.
> As for the content of the change itself, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>
Since the adjustments will be significant, I'll have to drop that
anyway, but thanks.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 15:38 ` Andrew Cooper
2016-07-01 15:54 ` Jan Beulich
@ 2016-07-01 16:10 ` Jan Beulich
2016-07-01 16:51 ` Andrew Cooper
1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-07-01 16:10 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 01.07.16 at 17:38, <andrew.cooper3@citrix.com> wrote:
> As for interleaving inside the asm statement itself, we already have
> precedent for that with the HAVE_GAS_* predicates. It would make the
> patch rather larger, but might end up looking cleaner. It is probably
> also worth switching to named parameters to reduce the risk of getting
> positional parameters out of order.
So taking just the first example I've converted: Do you think this
static bool_t even_parity(uint8_t v)
{
asm ( "test %1,%1"
#ifdef __GCC_ASM_FLAG_OUTPUTS__
: "=@ccp" (v)
#else
"; setp %0"
: "=qm" (v)
#endif
: "q" (v) );
return v;
}
is better than the original? I'm unsure, and I'm actually inclined to
think that then the abstraction alternative might look better.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 16:10 ` Jan Beulich
@ 2016-07-01 16:51 ` Andrew Cooper
2016-07-04 7:34 ` Jan Beulich
2016-08-01 16:06 ` Jan Beulich
0 siblings, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-07-01 16:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Kevin Tian, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 1831 bytes --]
On 01/07/16 17:10, Jan Beulich wrote:
>>>> On 01.07.16 at 17:38, <andrew.cooper3@citrix.com> wrote:
>> As for interleaving inside the asm statement itself, we already have
>> precedent for that with the HAVE_GAS_* predicates. It would make the
>> patch rather larger, but might end up looking cleaner. It is probably
>> also worth switching to named parameters to reduce the risk of getting
>> positional parameters out of order.
> So taking just the first example I've converted: Do you think this
>
> static bool_t even_parity(uint8_t v)
> {
> asm ( "test %1,%1"
> #ifdef __GCC_ASM_FLAG_OUTPUTS__
> : "=@ccp" (v)
> #else
> "; setp %0"
> : "=qm" (v)
> #endif
> : "q" (v) );
>
> return v;
> }
>
> is better than the original?
How about a different example, from the second hunk
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 460d1f7..8d52a41 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -832,8 +832,19 @@ static int read_ulong(
static bool_t mul_dbl(unsigned long m[2])
{
bool_t rc;
- asm ( "mul %1; seto %2"
- : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
+
+ asm ( "mul %1;"
+#ifndef __GCC_ASM_FLAG_OUTPUTS__
+ "seto %[rc];"
+#endif
+ : "+a" (m[0]), "+d" (m[1]),
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+ [rc] "=@cco" (rc)
+#else
+ [rc] "=qm" (rc)
+#endif
+ );
+
return rc;
}
This at least doesn't mix the : inside an #ifdef
> I'm unsure, and I'm actually inclined to
> think that then the abstraction alternative might look better.
If the abstraction comes in two parts, one which may insert a `setcc`
instruction, and one which selects between =qm and =@cc, it wouldn't end
up hiding the :.
~Andrew
[-- Attachment #1.2: Type: text/html, Size: 2879 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 15:01 [PATCH] x86: use gcc6'es flags asm() output support Jan Beulich
2016-07-01 15:38 ` Andrew Cooper
@ 2016-07-04 6:24 ` Tian, Kevin
1 sibling, 0 replies; 12+ messages in thread
From: Tian, Kevin @ 2016-07-04 6:24 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Andrew Cooper, Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, July 01, 2016 11:02 PM
>
> ..., rendering affected code more efficient and smaller.
>
> Note that in atomic.h this at once does away with the redundant output
> and input specifications of the memory location touched.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Do we want to abstract the pattern
> asm ( "...; set<cc> %<out>" : "=<cons>" (var) ... )
> matching
> asm ( "..." : "=@cc<cc>" (var) ... )
> via some macro? While this would eliminate many (all?) of the
> conditionals added here, it would result in the : no longer being
> visible in the actual source, making the asm()s look somewhat odd.
> Otherwise, to limit code duplication, it may be preferable to put
> the #ifdef-s inside the asm()s instead of around them.
>
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -406,12 +406,17 @@ static inline bool_t __vmread_safe(unsig
> VMREAD_OPCODE MODRM_EAX_ECX
> #endif
> /* CF==1 or ZF==1 --> rc = 0 */
> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
> + : "=@ccnbe" (okay),
> +#else
> "setnbe %0"
> + : "=qm" (okay),
> +#endif
> #ifdef HAVE_GAS_VMX
> - : "=qm" (okay), "=rm" (*value)
> + "=rm" (*value)
> : "r" (field));
> #else
> - : "=qm" (okay), "=c" (*value)
> + "=c" (*value)
> : "a" (field));
> #endif
>
>
Acked-by: Kevin Tian <kevin.tian@intel.com> for VMX part.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 16:51 ` Andrew Cooper
@ 2016-07-04 7:34 ` Jan Beulich
2016-08-01 16:06 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-07-04 7:34 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
> On 01/07/16 17:10, Jan Beulich wrote:
>>>>> On 01.07.16 at 17:38, <andrew.cooper3@citrix.com> wrote:
>>> As for interleaving inside the asm statement itself, we already have
>>> precedent for that with the HAVE_GAS_* predicates. It would make the
>>> patch rather larger, but might end up looking cleaner. It is probably
>>> also worth switching to named parameters to reduce the risk of getting
>>> positional parameters out of order.
>> So taking just the first example I've converted: Do you think this
>>
>> static bool_t even_parity(uint8_t v)
>> {
>> asm ( "test %1,%1"
>> #ifdef __GCC_ASM_FLAG_OUTPUTS__
>> : "=@ccp" (v)
>> #else
>> "; setp %0"
>> : "=qm" (v)
>> #endif
>> : "q" (v) );
>>
>> return v;
>> }
>>
>> is better than the original?
>
> How about a different example, from the second hunk
>
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 460d1f7..8d52a41 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -832,8 +832,19 @@ static int read_ulong(
> static bool_t mul_dbl(unsigned long m[2])
> {
> bool_t rc;
> - asm ( "mul %1; seto %2"
> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
> +
> + asm ( "mul %1;"
> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
> + "seto %[rc];"
> +#endif
> + : "+a" (m[0]), "+d" (m[1]),
> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
> + [rc] "=@cco" (rc)
> +#else
> + [rc] "=qm" (rc)
> +#endif
> + );
> +
> return rc;
> }
>
> This at least doesn't mix the : inside an #ifdef
At the price of two #ifdef-s. And in the example I'm really not
worried about the colon going into both branches of the #if, but
about general readability of the resulting code.
>> I'm unsure, and I'm actually inclined to
>> think that then the abstraction alternative might look better.
>
> If the abstraction comes in two parts, one which may insert a `setcc`
> instruction, and one which selects between =qm and =@cc, it wouldn't end
> up hiding the :.
Opening an easy route to making mistakes. Imo such an abstraction
needs to be either a single item, ot not be done at all.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-07-01 16:51 ` Andrew Cooper
2016-07-04 7:34 ` Jan Beulich
@ 2016-08-01 16:06 ` Jan Beulich
2016-08-01 17:11 ` Andrew Cooper
1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-08-01 16:06 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
> How about a different example, from the second hunk
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -832,8 +832,19 @@ static int read_ulong(
> static bool_t mul_dbl(unsigned long m[2])
> {
> bool_t rc;
> - asm ( "mul %1; seto %2"
> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
> +
> + asm ( "mul %1;"
> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
> + "seto %[rc];"
> +#endif
> + : "+a" (m[0]), "+d" (m[1]),
> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
> + [rc] "=@cco" (rc)
> +#else
> + [rc] "=qm" (rc)
> +#endif
> + );
> +
> return rc;
> }
Looking at this again I think I really like the original, submitted version
better. Are you strongly biased towards the above form?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-08-01 16:06 ` Jan Beulich
@ 2016-08-01 17:11 ` Andrew Cooper
2016-08-02 6:09 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-08-01 17:11 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Kevin Tian, Jun Nakajima
On 01/08/16 17:06, Jan Beulich wrote:
>>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
>> How about a different example, from the second hunk
>>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -832,8 +832,19 @@ static int read_ulong(
>> static bool_t mul_dbl(unsigned long m[2])
>> {
>> bool_t rc;
>> - asm ( "mul %1; seto %2"
>> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
>> +
>> + asm ( "mul %1;"
>> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
>> + "seto %[rc];"
>> +#endif
>> + : "+a" (m[0]), "+d" (m[1]),
>> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
>> + [rc] "=@cco" (rc)
>> +#else
>> + [rc] "=qm" (rc)
>> +#endif
>> + );
>> +
>> return rc;
>> }
> Looking at this again I think I really like the original, submitted version
> better. Are you strongly biased towards the above form?
I am not overly fussed between this version and the original submission.
However, I definitely think that we shouldn't hide semantic bits of the
ASM statement behind macros.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-08-01 17:11 ` Andrew Cooper
@ 2016-08-02 6:09 ` Jan Beulich
2016-08-02 9:15 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-08-02 6:09 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 01.08.16 at 19:11, <andrew.cooper3@citrix.com> wrote:
> On 01/08/16 17:06, Jan Beulich wrote:
>>>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
>>> How about a different example, from the second hunk
>>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -832,8 +832,19 @@ static int read_ulong(
>>> static bool_t mul_dbl(unsigned long m[2])
>>> {
>>> bool_t rc;
>>> - asm ( "mul %1; seto %2"
>>> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
>>> +
>>> + asm ( "mul %1;"
>>> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
>>> + "seto %[rc];"
>>> +#endif
>>> + : "+a" (m[0]), "+d" (m[1]),
>>> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
>>> + [rc] "=@cco" (rc)
>>> +#else
>>> + [rc] "=qm" (rc)
>>> +#endif
>>> + );
>>> +
>>> return rc;
>>> }
>> Looking at this again I think I really like the original, submitted version
>> better. Are you strongly biased towards the above form?
>
> I am not overly fussed between this version and the original submission.
>
> However, I definitely think that we shouldn't hide semantic bits of the
> ASM statement behind macros.
Well, the originally submitted variant doesn't do anything like that,
so may I translate the above to an ack?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-08-02 6:09 ` Jan Beulich
@ 2016-08-02 9:15 ` Andrew Cooper
2016-08-02 10:39 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-08-02 9:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Kevin Tian, Jun Nakajima
On 02/08/16 07:09, Jan Beulich wrote:
>>>> On 01.08.16 at 19:11, <andrew.cooper3@citrix.com> wrote:
>> On 01/08/16 17:06, Jan Beulich wrote:
>>>>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
>>>> How about a different example, from the second hunk
>>>>
>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> @@ -832,8 +832,19 @@ static int read_ulong(
>>>> static bool_t mul_dbl(unsigned long m[2])
>>>> {
>>>> bool_t rc;
>>>> - asm ( "mul %1; seto %2"
>>>> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
>>>> +
>>>> + asm ( "mul %1;"
>>>> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
>>>> + "seto %[rc];"
>>>> +#endif
>>>> + : "+a" (m[0]), "+d" (m[1]),
>>>> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
>>>> + [rc] "=@cco" (rc)
>>>> +#else
>>>> + [rc] "=qm" (rc)
>>>> +#endif
>>>> + );
>>>> +
>>>> return rc;
>>>> }
>>> Looking at this again I think I really like the original, submitted version
>>> better. Are you strongly biased towards the above form?
>> I am not overly fussed between this version and the original submission.
>>
>> However, I definitely think that we shouldn't hide semantic bits of the
>> ASM statement behind macros.
> Well, the originally submitted variant doesn't do anything like that,
> so may I translate the above to an ack?
You do already have a Rev-by from my first reply.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86: use gcc6'es flags asm() output support
2016-08-02 9:15 ` Andrew Cooper
@ 2016-08-02 10:39 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-08-02 10:39 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Jun Nakajima
>>> On 02.08.16 at 11:15, <andrew.cooper3@citrix.com> wrote:
> On 02/08/16 07:09, Jan Beulich wrote:
>>>>> On 01.08.16 at 19:11, <andrew.cooper3@citrix.com> wrote:
>>> On 01/08/16 17:06, Jan Beulich wrote:
>>>>>>> On 01.07.16 at 18:51, <andrew.cooper3@citrix.com> wrote:
>>>>> How about a different example, from the second hunk
>>>>>
>>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>>>> @@ -832,8 +832,19 @@ static int read_ulong(
>>>>> static bool_t mul_dbl(unsigned long m[2])
>>>>> {
>>>>> bool_t rc;
>>>>> - asm ( "mul %1; seto %2"
>>>>> - : "+a" (m[0]), "+d" (m[1]), "=qm" (rc) );
>>>>> +
>>>>> + asm ( "mul %1;"
>>>>> +#ifndef __GCC_ASM_FLAG_OUTPUTS__
>>>>> + "seto %[rc];"
>>>>> +#endif
>>>>> + : "+a" (m[0]), "+d" (m[1]),
>>>>> +#ifdef __GCC_ASM_FLAG_OUTPUTS__
>>>>> + [rc] "=@cco" (rc)
>>>>> +#else
>>>>> + [rc] "=qm" (rc)
>>>>> +#endif
>>>>> + );
>>>>> +
>>>>> return rc;
>>>>> }
>>>> Looking at this again I think I really like the original, submitted version
>>>> better. Are you strongly biased towards the above form?
>>> I am not overly fussed between this version and the original submission.
>>>
>>> However, I definitely think that we shouldn't hide semantic bits of the
>>> ASM statement behind macros.
>> Well, the originally submitted variant doesn't do anything like that,
>> so may I translate the above to an ack?
>
> You do already have a Rev-by from my first reply.
Oops, indeed. I'm sorry for the noise then.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-08-02 10:39 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01 15:01 [PATCH] x86: use gcc6'es flags asm() output support Jan Beulich
2016-07-01 15:38 ` Andrew Cooper
2016-07-01 15:54 ` Jan Beulich
2016-07-01 16:10 ` Jan Beulich
2016-07-01 16:51 ` Andrew Cooper
2016-07-04 7:34 ` Jan Beulich
2016-08-01 16:06 ` Jan Beulich
2016-08-01 17:11 ` Andrew Cooper
2016-08-02 6:09 ` Jan Beulich
2016-08-02 9:15 ` Andrew Cooper
2016-08-02 10:39 ` Jan Beulich
2016-07-04 6:24 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.