[Qemu-devel] [PATCH for-1.7] atomic.h: Fix build with clang

[Qemu-devel] [PATCH for-1.7] atomic.h: Fix build with clang

[Peter Maydell]

clang defines __ATOMIC_SEQ_CST but its implementation of the
__atomic_exchange() builtin differs from that of gcc. Move the
__clang__ branch of the ifdef ladder to the top and fix its
implementation (there is no such builtin as __sync_exchange),
so we can compile with clang again.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I don't have access to a Linux box with clang, so this is only
tested on MacOSX clang, but I believe it to be both required
and correct for all clang platforms.

 include/qemu/atomic.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 0aa8913..492bce1 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -168,14 +168,14 @@
 #endif
 
 #ifndef atomic_xchg
-#ifdef __ATOMIC_SEQ_CST
+#if defined(__clang__)
+#define atomic_xchg(ptr, i)    __sync_swap(ptr, i)
+#elif defined(__ATOMIC_SEQ_CST)
 #define atomic_xchg(ptr, i)    ({                           \
     typeof(*ptr) _new = (i), _old;                          \
     __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
     _old;                                                   \
 })
-#elif defined __clang__
-#define atomic_xchg(ptr, i)    __sync_exchange(ptr, i)
 #else
 /* __sync_lock_test_and_set() is documented to be an acquire barrier only.  */
 #define atomic_xchg(ptr, i)    (smp_mb(), __sync_lock_test_and_set(ptr, i))
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH for-1.7] atomic.h: Fix build with clang

[Peter Maydell]

Ping! This is needed as a build-fix for MacOSX and didn't
make it into 1.7-rc0. Paolo, can I get you to review this?

I did subsequently test with a Linux clang 3.3: the patch
works OK on that but is not necessary because that clang
doesn't define __ATOMIC_SEQ_CST, presumably because
it's not recent enough. I imagine recent Linux clang still needs
this fix.

thanks
-- PMM

On 22 October 2013 10:58, Peter Maydell <peter.maydell@linaro.org> wrote:
> clang defines __ATOMIC_SEQ_CST but its implementation of the
> __atomic_exchange() builtin differs from that of gcc. Move the
> __clang__ branch of the ifdef ladder to the top and fix its
> implementation (there is no such builtin as __sync_exchange),
> so we can compile with clang again.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't have access to a Linux box with clang, so this is only
> tested on MacOSX clang, but I believe it to be both required
> and correct for all clang platforms.
>
>  include/qemu/atomic.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index 0aa8913..492bce1 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -168,14 +168,14 @@
>  #endif
>
>  #ifndef atomic_xchg
> -#ifdef __ATOMIC_SEQ_CST
> +#if defined(__clang__)
> +#define atomic_xchg(ptr, i)    __sync_swap(ptr, i)
> +#elif defined(__ATOMIC_SEQ_CST)
>  #define atomic_xchg(ptr, i)    ({                           \
>      typeof(*ptr) _new = (i), _old;                          \
>      __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
>      _old;                                                   \
>  })
> -#elif defined __clang__
> -#define atomic_xchg(ptr, i)    __sync_exchange(ptr, i)
>  #else
>  /* __sync_lock_test_and_set() is documented to be an acquire barrier only.  */
>  #define atomic_xchg(ptr, i)    (smp_mb(), __sync_lock_test_and_set(ptr, i))
> --
> 1.7.11.4

Re: [Qemu-devel] [PATCH for-1.7] atomic.h: Fix build with clang

[Paolo Bonzini]

Il 09/11/2013 19:09, Peter Maydell ha scritto:
> Ping! This is needed as a build-fix for MacOSX and didn't
> make it into 1.7-rc0. Paolo, can I get you to review this?

I thought I already had done that, anyway:

>> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
>> index 0aa8913..492bce1 100644
>> --- a/include/qemu/atomic.h
>> +++ b/include/qemu/atomic.h
>> @@ -168,14 +168,14 @@
>>  #endif
>>
>>  #ifndef atomic_xchg
>> -#ifdef __ATOMIC_SEQ_CST
>> +#if defined(__clang__)
>> +#define atomic_xchg(ptr, i)    __sync_swap(ptr, i)
>> +#elif defined(__ATOMIC_SEQ_CST)
>>  #define atomic_xchg(ptr, i)    ({                           \
>>      typeof(*ptr) _new = (i), _old;                          \
>>      __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
>>      _old;                                                   \
>>  })
>> -#elif defined __clang__
>> -#define atomic_xchg(ptr, i)    __sync_exchange(ptr, i)
>>  #else
>>  /* __sync_lock_test_and_set() is documented to be an acquire barrier only.  */
>>  #define atomic_xchg(ptr, i)    (smp_mb(), __sync_lock_test_and_set(ptr, i))

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>