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

[Ilia Mirkin]

On Mon, Nov 17, 2014 at 2:15 PM, Patrick Baggett
<baggett.patrick at gmail.com> wrote:
>
>
> On Mon, Nov 17, 2014 at 12: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" )
>> +
>> +static INLINE int32_t
>> +p_atomic_inc_return(int32_t *v)
>> +{
>> +   (void) v;
>> +   assert(0);
>> +   return 0;
>> +}
>
>
> Why isn't _InterlockedIncrement() used here? It is used for the void
> functions. If you read the definition of _InterlockedIncrement() it returns
> the new value -- isn't that what is needed?

Looking at u_atomic.h there is a section that uses
PIPE_ATOMIC_ASM_MSVC_X86 and has explicit assembly, and there's a
section that uses PIPE_ATOMIC_MSVC_INTRINSIC and has intrinsics. No
clue whatsoever what the difference between them is, but presumably it
doesn't exist solely for the purpose of annoying developers...

  -ilia