Re: [Qemu-devel] [PATCH v2] atomics: Use __atomic_*_n() variant primitives

[Paolo Bonzini <pbonzini@redhat.com>]

On 29/08/2016 19:17, Pranith Kumar wrote:
> Use the __atomic_*_n() primitives which take the value as argument. It
> is not necessary to store the value locally before calling the
> primitive, hence saving us a stack store and load.
> 
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
>  include/qemu/atomic.h | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> v2:
>  - convert atomic_load() and atomic_compare_exchange()
> 
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index 43b0645..9bb991e 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -96,15 +96,12 @@
>  #define atomic_read(ptr)                              \
>      ({                                                \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> -    typeof_strip_qual(*ptr) _val;                     \
> -     __atomic_load(ptr, &_val, __ATOMIC_RELAXED);     \
> -    _val;                                             \
> +    __atomic_load_n(ptr, __ATOMIC_RELAXED);           \
>      })
>  
>  #define atomic_set(ptr, i)  do {                      \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> -    typeof(*ptr) _val = (i);                          \
> -    __atomic_store(ptr, &_val, __ATOMIC_RELAXED);     \
> +    __atomic_store_n(ptr, i, __ATOMIC_RELAXED);       \
>  } while(0)
>  
>  /* See above: most compilers currently treat consume and acquire the
> @@ -129,8 +126,7 @@
>  
>  #define atomic_rcu_set(ptr, i) do {                   \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> -    typeof(*ptr) _val = (i);                          \
> -    __atomic_store(ptr, &_val, __ATOMIC_RELEASE);     \
> +    __atomic_store_n(ptr, i, __ATOMIC_RELEASE);       \
>  } while(0)
>  
>  /* atomic_mb_read/set semantics map Java volatile variables. They are
> @@ -153,9 +149,8 @@
>  
>  #define atomic_mb_set(ptr, i)  do {                     \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
> -    typeof(*ptr) _val = (i);                            \
>      smp_wmb();                                          \
> -    __atomic_store(ptr, &_val, __ATOMIC_RELAXED);       \
> +    __atomic_store_n(ptr, i, __ATOMIC_RELAXED);         \
>      smp_mb();                                           \
>  } while(0)
>  #else
> @@ -169,8 +164,7 @@
>  
>  #define atomic_mb_set(ptr, i)  do {                     \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
> -    typeof(*ptr) _val = (i);                            \
> -    __atomic_store(ptr, &_val, __ATOMIC_SEQ_CST);       \
> +    __atomic_store_n(ptr, i, __ATOMIC_SEQ_CST);         \
>  } while(0)
>  #endif
>  
> @@ -179,17 +173,15 @@
>  
>  #define atomic_xchg(ptr, i)    ({                           \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));       \
> -    typeof_strip_qual(*ptr) _new = (i), _old;               \
> -    __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
> -    _old;                                                   \
> +    __atomic_exchange_n(ptr, i, __ATOMIC_SEQ_CST);          \
>  })
>  
>  /* Returns the eventual value, failed or not */
>  #define atomic_cmpxchg(ptr, old, new)                                   \
>      ({                                                                  \
>      QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));                   \
> -    typeof_strip_qual(*ptr) _old = (old), _new = (new);                 \
> -    __atomic_compare_exchange(ptr, &_old, &_new, false,                 \
> +    typeof_strip_qual(*ptr) _old = (old);                               \
> +    __atomic_compare_exchange_n(ptr, &_old, new, false,                 \
>                                __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);      \
>      _old;                                                               \
>      })
> 

Thanks, looks good.

Paolo