[Mesa-dev] [PATCH 2/2] util/u_atomic: Add _InterlockedExchangeAdd8/16 for older MSVC.

[Jose Fonseca]

On 12/02/15 17:03, Brian Paul wrote:
> On 02/12/2015 09:27 AM, Jose Fonseca wrote:
>> We need to build certain parts of Mesa (namely gallium, llvmpipe, and
>> therefore util) with Windows SDK 7.0.7600, which includes MSVC 2008.
>> ---
>>   src/util/u_atomic.h | 32 ++++++++++++++++++++++++++++++--
>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/u_atomic.h b/src/util/u_atomic.h
>> index e123e17..f54def3 100644
>> --- a/src/util/u_atomic.h
>> +++ b/src/util/u_atomic.h
>> @@ -88,7 +88,7 @@
>>
>>   #if _MSC_VER < 1600
>>
>> -/* Implement _InterlockedCompareExchange8 in terms of
>> InterlockedCompareExchange16 */
>> +/* Implement _InterlockedCompareExchange8 in terms of
>> _InterlockedCompareExchange16 */
>>   static __inline
>>   char _InterlockedCompareExchange8(char volatile *Destination8, char
>> Exchange8, char Comparand8)
>
> static __inline char
> _InterlockedCompareExchange8(...)
>
>
>>   {
>> @@ -103,7 +103,7 @@ char _InterlockedCompareExchange8(char volatile
>> *Destination8, char Exchange8, c
>>          * neighboring byte untouched */
>>         short Exchange16 = (Initial16 & ~Mask8) | ((short)Exchange8 <<
>> Shift8);
>>         short Comparand16 = Initial16;
>> -      short Initial16 = InterlockedCompareExchange16(Destination16,
>> Exchange16, Comparand16);
>> +      short Initial16 = _InterlockedCompareExchange16(Destination16,
>> Exchange16, Comparand16);
>>         if (Initial16 == Comparand16) {
>>            /* succeeded */
>>            return Comparand8;
>> @@ -114,6 +114,34 @@ char _InterlockedCompareExchange8(char volatile
>> *Destination8, char Exchange8, c
>>      return Initial8;
>>   }
>>
>> +/* Implement _InterlockedExchangeAdd16 in terms of
>> _InterlockedCompareExchange16 */
>> +static __inline
>> +short _InterlockedExchangeAdd16(short volatile *Addend, short Value)
>
> same thing.
>
>> +{
>> +   short Initial = *Addend;
>> +   short Comparand;
>> +   do {
>> +      short Exchange = Initial + Value;
>> +      Comparand = Initial;
>> +      Initial = _InterlockedCompareExchange16(Addend, Exchange,
>> Comparand);
>> +   } while(Initial != Comparand);
>> +   return Comparand;
>> +}
>
> I had to stare at this for quite a while.  It's kind of a mind bender
> (at least to me).  I found it helpful to add a comment:
>
>        /* if *Addend==Comparand then *Addend=Exchange, return original
> *Addend */
>
> before the _InterlockedCompareExchange16() call.
>
>
>> +
>> +/* Implement _InterlockedExchangeAdd8 in terms of
>> _InterlockedCompareExchange8 */
>> +static __inline
>> +char _InterlockedExchangeAdd8(char volatile *Addend, char Value)
>> +{
>> +   char Initial = *Addend;
>> +   char Comparand;
>> +   do {
>> +      char Exchange = Initial + Value;
>> +      Comparand = Initial;
>> +      Initial = _InterlockedCompareExchange8(Addend, Exchange,
>> Comparand);
>> +   } while(Initial != Comparand);
>> +   return Comparand;
>> +}
>> +
>>   #endif /* _MSC_VER < 1600 */
>>
>>   /* MSVC supports decltype keyword, but it's only supported on C++
>> and doesn't
>>

Thanks for the review.  I'll change as you suggested.

> Why do the local variables start with upper case letters?  That's not
> our usual style.

It matches the prototype documentation on MSDN -- whenever I work with 
Windows APIs I instictively switch to Microsoft's style.

But I agree readability of Caps var is not great. I'll push a follow on 
change switching vars to lower caps.

> Anyway,
> Reviewed-by: Brian Paul <brianp at vmware.com>
>


Jose