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

Submitted by Jose Fonseca on Feb. 12, 2015, 4:27 p.m.

Details

Message ID 1423758422-30374-2-git-send-email-jfonseca@vmware.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Jose Fonseca Feb. 12, 2015, 4:27 p.m.
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(-)

Patch hide | download patch | download mbox

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)
 {
@@ -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)
+{
+   short Initial = *Addend;
+   short Comparand;
+   do {
+      short Exchange = Initial + Value;
+      Comparand = Initial;
+      Initial = _InterlockedCompareExchange16(Addend, Exchange, Comparand);
+   } while(Initial != Comparand);
+   return Comparand;
+}
+
+/* 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

Comments

Series looks good to me.

Roland

Am 12.02.2015 um 17:27 schrieb Jose Fonseca:
> 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)
>  {
> @@ -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)
> +{
> +   short Initial = *Addend;
> +   short Comparand;
> +   do {
> +      short Exchange = Initial + Value;
> +      Comparand = Initial;
> +      Initial = _InterlockedCompareExchange16(Addend, Exchange, Comparand);
> +   } while(Initial != Comparand);
> +   return Comparand;
> +}
> +
> +/* 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
>
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
>

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

Anyway,
Reviewed-by: Brian Paul <brianp@vmware.com>
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@vmware.com>
>


Jose