stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] _mutex.h
Date Wed, 02 Jan 2008 18:41:19 GMT
Farid Zaripov wrote:
>   The one of the problems in boost regression tests on MSVC 7.1 was caused by #definition
> _InterlockedIncrement as InterlockedIncrement. The patch below fixes the problem. The
MSVC 7.1
> also has the intrinsic interlocked functions, but the are not declared in any header
files (in MSVC 8.0
> and higher they are declared in <intrin.h>). The patch below resolves this problem.

Makes sense. A couple of questions/comments below...

> 
>  
>   ChangeLog:
>   * include/rw/_config-msvcrt.h: Don't #define _RWSTD_NO_LONG_LONG as __int64
>   if long long type is supported by compiler.
>   * include/rw/_mutex.h: Use intrinsic interlocked functions on MSVC 7.1 and ICC.
>   Declare the interlocked functions instead of #including <intrin.h>.
>  
>  
> Index: include/rw/_config-msvcrt.h
> ===================================================================
> --- include/rw/_config-msvcrt.h (revision 607181)
> +++ include/rw/_config-msvcrt.h (working copy)
> @@ -89,7 +89,9 @@
>  #endif   // _RWSTD_NO_STATIC_CONST_MEMBER_DEFINITION
>  
>     // enable iostream and locale support for long long integers
> -#define _RWSTD_LONG_LONG __int64
> +#ifdef _RWSTD_NO_LONG_LONG
> +#  define _RWSTD_LONG_LONG __int64
> +#endif
>  
>  #if defined (_WIN64)
>       // FIXME: handle by forward declaring fuctions in <rw/_mutex.h>
> Index: include/rw/_mutex.h
> ===================================================================
> --- include/rw/_mutex.h (revision 607181)
> +++ include/rw/_mutex.h (working copy)
> @@ -119,6 +119,12 @@
>  #    include <windows.h>
>  #    define _RWSTD_MUTEX_T _RTL_CRITICAL_SECTION
>  
> +#    ifndef _MSC_VER
> +#      define _InterlockedIncrement InterlockedIncrement
> +#      define _InterlockedDecrement InterlockedDecrement
> +#      define _InterlockedExchange  InterlockedExchange
> +#    endif    // _MSC_VER
> +
>  #  else   // if defined (_RWSTD_NO_FWD_DECLARATIONS)
>  
>     // avoid #including this header (MFC doesn't like it)
> @@ -142,7 +148,7 @@
>  DeleteCriticalSection (_RTL_CRITICAL_SECTION*);
>  
>  
> -#if defined _RWSTD_INTERLOCKED_T && (!defined (_MSC_VER) || _MSC_VER < 1400)
> +#if defined (_RWSTD_INTERLOCKED_T) && !defined (_MSC_VER)
>  
>  __declspec (dllimport) long __stdcall
>  InterlockedIncrement (_RWSTD_INTERLOCKED_T*);
> @@ -157,7 +163,7 @@
>  #  define _InterlockedDecrement InterlockedDecrement
>  #  define _InterlockedExchange  InterlockedExchange
>  
> -#endif   // _RWSTD_INTERLOCKED_T && (!_MSC_VER || _MSC_VER < 1400)
> +#endif   // _RWSTD_INTERLOCKED_T && !_MSC_VER
>  
>  }   // extern "C"
>  
> @@ -176,21 +182,35 @@
>  
>  #  endif   // _RWSTD_NO_FWD_DECLARATIONS
>  
> -#  if defined (_MSC_VER) && _MSC_VER >= 1400 && !defined (__INTEL_COMPILER)
> -#    include <intrin.h>
> +#  ifdef _MSC_VER
> +extern "C" long __cdecl _InterlockedIncrement (long volatile*);
> +extern "C" long __cdecl _InterlockedDecrement (long volatile*);
> +extern "C" long __cdecl _InterlockedExchange (long volatile*, long);

Shouldn't the type of the argument be _RWSTD_INTERLOCKED_T* instead?

If not, our convention is to put the cv-qualifier(s) before the type,
not after it (i.e., const T* or volatile T*, rather than T const* or
T volatile*).

Martin

> +#    ifndef __INTEL_COMPILER
> +#      pragma intrinsic (_InterlockedIncrement)
> +#      pragma intrinsic (_InterlockedDecrement)
> +#      pragma intrinsic (_InterlockedExchange)
> +#    endif   // __INTEL_COMPILER
>  
> -#    pragma intrinsic (_InterlockedIncrement)
> -#    pragma intrinsic (_InterlockedIncrement16)
> -#    pragma intrinsic (_InterlockedDecrement)
> -#    pragma intrinsic (_InterlockedDecrement16)
> -#    pragma intrinsic (_InterlockedExchange)
> +#    if _MSC_VER >= 1400 && !defined (__INTEL_COMPILER)
> +extern "C" short __cdecl _InterlockedIncrement16 (short volatile*);
> +extern "C" short __cdecl _InterlockedDecrement16 (short volatile*);
> +#      pragma intrinsic (_InterlockedIncrement16)
> +#      pragma intrinsic (_InterlockedDecrement16)
> +#    endif   // _MSC_VER >= 1400 && !__INTEL_COMPILER
>  
>  #    ifdef _M_X64
> -#      pragma intrinsic (_InterlockedIncrement64)
> -#      pragma intrinsic (_InterlockedDecrement64)
> -#      pragma intrinsic (_InterlockedExchange64)
> -#    endif
> -#  endif   // _MSC_VER >= 1400 && !__INTEL_COMPILER
> +extern "C" long long __cdecl _InterlockedIncrement64 (long long volatile*);
> +extern "C" long long __cdecl _InterlockedDecrement64 (long long volatile*);
> +extern "C" long long __cdecl _InterlockedExchange64 (long long volatile*,
> +                                                     long long);
> +#      ifndef __INTEL_COMPILER
> +#        pragma intrinsic (_InterlockedIncrement64)
> +#        pragma intrinsic (_InterlockedDecrement64)
> +#        pragma intrinsic (_InterlockedExchange64)
> +#      endif   // __INTEL_COMPILER
> +#    endif   // _M_X64
> +#  endif   // _MSC_VER
>  
>  
>  _RWSTD_NAMESPACE (__rw) { 
> 


Mime
View raw message