[Mesa-dev] [PATCH v5] gallium/auxiliary: add inc and dec alternative with return (v3)

[Ilia Mirkin]

On Mon, Nov 17, 2014 at 1:20 PM, Axel Davy <axel.davy at ens.fr> wrote:
> From: Christoph Bumiller <christoph.bumiller at speed.at>
>
> At this moment we use only zero or positive values.
>
> v2: Implement it for also for Solaris, MSVC assembly
>     and enable for other combinations.
>
> v3: Replace MSVC assembly by assert + warning during compilation
>
> Signed-off-by: David Heidelberg <david at ixit.cz>
> ---
>  src/gallium/auxiliary/util/u_atomic.h | 72 +++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_atomic.h b/src/gallium/auxiliary/util/u_atomic.h
> index 2f2b42b..9279073 100644
> --- a/src/gallium/auxiliary/util/u_atomic.h
> +++ b/src/gallium/auxiliary/util/u_atomic.h
> @@ -69,6 +69,18 @@ p_atomic_dec(int32_t *v)
>  }
>
>  static INLINE int32_t
> +p_atomic_inc_return(int32_t *v)
> +{
> +   return __sync_add_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
> +p_atomic_dec_return(int32_t *v)
> +{
> +   return __sync_sub_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
>  p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  {
>     return __sync_val_compare_and_swap(v, old, _new);
> @@ -116,6 +128,18 @@ p_atomic_dec(int32_t *v)
>  }
>
>  static INLINE int32_t
> +p_atomic_inc_return(int32_t *v)
> +{
> +   return __sync_add_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
> +p_atomic_dec_return(int32_t *v)
> +{
> +   return __sync_sub_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
>  p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  {
>     return __sync_val_compare_and_swap(v, old, _new);
> @@ -161,6 +185,18 @@ p_atomic_dec(int32_t *v)
>  }
>
>  static INLINE int32_t
> +p_atomic_inc_return(int32_t *v)
> +{
> +   return __sync_add_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
> +p_atomic_dec_return(int32_t *v)
> +{
> +   return __sync_sub_and_fetch(v, 1);
> +}
> +
> +static INLINE int32_t
>  p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  {
>     return __sync_val_compare_and_swap(v, old, _new);
> @@ -186,6 +222,8 @@ p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  #define p_atomic_dec_zero(_v) ((boolean) --(*(_v)))
>  #define p_atomic_inc(_v) ((void) (*(_v))++)
>  #define p_atomic_dec(_v) ((void) (*(_v))--)
> +#define p_atomic_inc_return(_v) ((*(_v))++)
> +#define p_atomic_dec_return(_v) ((*(_v))--)
>  #define p_atomic_cmpxchg(_v, old, _new) (*(_v) == old ? *(_v) = (_new) : *(_v))
>
>  #endif
> @@ -197,6 +235,8 @@ p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>
>  #define PIPE_ATOMIC "MSVC x86 assembly"
>
> +#include <assert.h>
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -236,6 +276,24 @@ p_atomic_dec(int32_t *v)
>     }
>  }
>
> +#pragma message ( "Warning: p_atomic_dec_return and p_atomic_inc_return unimplemented for PIPE_ATOMIC_ASM_MSVC_X86" )

... but nothing uses these functions that gets compiled by this
compiler. So you get a warning message on anything that includes
atomics (i.e. everything), but there's no actual issue. This seems
suboptimal.

What about just not having the functions there at all? Would that
cause any issues (unless someone tried to build st/nine)? I guess the
concern is that it would allow someone to foolishly start using these
primitives in MSVC-compiled code and then the vmware guys would be
left picking up the pieces.

OTOH, it seems fair to get them to provide the correct implementation
since they're the ones that care about the platform.

OTTH, I've spent more time typing this e-mail than it would take to
look up how to do this properly, so... why not just do that? :)

> +
> +static INLINE int32_t
> +p_atomic_inc_return(int32_t *v)
> +{
> +   (void) v;
> +   assert(0);
> +   return 0;
> +}
> +
> +static INLINE int32_t
> +p_atomic_dec_return(int32_t *v)
> +{
> +   (void) v;
> +   assert(0);
> +   return 0;
> +}
> +
>  static INLINE int32_t
>  p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  {
> @@ -288,6 +346,12 @@ p_atomic_inc(int32_t *v)
>     _InterlockedIncrement((long *)v);
>  }
>
> +static INLINE int32_t
> +p_atomic_inc_return(int32_t *v)
> +{
> +   return _InterlockedIncrement((long *)v);
> +}
> +
>  static INLINE void
>  p_atomic_dec(int32_t *v)
>  {
> @@ -295,6 +359,12 @@ p_atomic_dec(int32_t *v)
>  }
>
>  static INLINE int32_t
> +p_atomic_dec_return(int32_t *v)
> +{
> +   return _InterlockedDecrement((long *)v);
> +}
> +
> +static INLINE int32_t
>  p_atomic_cmpxchg(int32_t *v, int32_t old, int32_t _new)
>  {
>     return _InterlockedCompareExchange((long *)v, _new, old);
> @@ -329,6 +399,8 @@ p_atomic_dec_zero(int32_t *v)
>
>  #define p_atomic_inc(_v) atomic_inc_32((uint32_t *) _v)
>  #define p_atomic_dec(_v) atomic_dec_32((uint32_t *) _v)
> +#define p_atomic_inc_return(_v) atomic_inc_32_nv((uint32_t *) _v)
> +#define p_atomic_dec_return(_v) atomic_dec_32_nv((uint32_t *) _v)
>
>  #define p_atomic_cmpxchg(_v, _old, _new) \
>         atomic_cas_32( (uint32_t *) _v, (uint32_t) _old, (uint32_t) _new)
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev