You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2008/01/02 19:41:19 UTC
Re: [PATCH] _mutex.h
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) {
>
Re: [PATCH] _mutex.h
Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Пн, 21.01.2008 18:21
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: [PATCH] _mutex.h
>
>>> No. _RWSTD_INTERLOCKED_T #defined only for 32-bit compiler, while intrinsic
>>> functions are available in 32 and 64-bit compilers.
>
>> Why isn't it defined for the 64-bit compiler? (Because the config
>> test fails?)
>
> The config test doesn't fails because the determining of the _RWSTD_INTERLOCKED_T
> type (ATOMIC_OPS.cpp config test) is guarded by the
> #if defined (_WIN32) && !defined (_WIN64) #else #endif. The rason is that
> __stdcall InterlockedXXX functiond are present only in Win32 API. The Win64 API
> doesn't provides theese functions, but the compiler supports __cdecl _InterlockedXXX
> intrinsic functions and windows.h contains #defines of the InterlockedXXX to _InterlockedXXX.
>
>> It seems that it should be defined regardless, wouldn't
>> you say? Even if we need to hardcode it _config.h or _defs.h...
>
> Actually the _RWSTD_INTERLOCKED_T define is used only in declaration of the InterlockedIncrement(),
>
> InterlockedDecrement() and InterlockedExchange() functions in rw/_mutex.h. But after the my latest patch
>
> these functions are not used, because of using the instrinsic _InterlockedXXX() functions instead on all supported
>
> compilers on Windows (except gcc/cygwin where are used __rw_atomic_xxx() functions).
So, _RWSTD_INTERLOCKED_T isn't used for any compiler anymore?
If that's the case, should we just get rid of it?
Martin
Re: [PATCH] _mutex.h
Posted by Farid Zaripov <Fa...@epam.com>.
From: Martin Sebor [mailto:sebor@roguewave.com]
Sent: Пн, 21.01.2008 18:21
To: stdcxx-dev@incubator.apache.org
Subject: Re: [PATCH] _mutex.h
>> No. _RWSTD_INTERLOCKED_T #defined only for 32-bit compiler, while intrinsic
>> functions are available in 32 and 64-bit compilers.
> Why isn't it defined for the 64-bit compiler? (Because the config
> test fails?)
The config test doesn't fails because the determining of the _RWSTD_INTERLOCKED_T
type (ATOMIC_OPS.cpp config test) is guarded by the
#if defined (_WIN32) && !defined (_WIN64) #else #endif. The rason is that
__stdcall InterlockedXXX functiond are present only in Win32 API. The Win64 API
doesn't provides theese functions, but the compiler supports __cdecl _InterlockedXXX
intrinsic functions and windows.h contains #defines of the InterlockedXXX to _InterlockedXXX.
> It seems that it should be defined regardless, wouldn't
> you say? Even if we need to hardcode it _config.h or _defs.h...
Actually the _RWSTD_INTERLOCKED_T define is used only in declaration of the InterlockedIncrement(),
InterlockedDecrement() and InterlockedExchange() functions in rw/_mutex.h. But after the my latest patch
these functions are not used, because of using the instrinsic _InterlockedXXX() functions instead on all supported
compilers on Windows (except gcc/cygwin where are used __rw_atomic_xxx() functions).
Farid.
Re: [PATCH] _mutex.h
Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Ср, 02.01.2008 20:41
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: [PATCH] _mutex.h
>
>>> +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?
>
> No. _RWSTD_INTERLOCKED_T #defined only for 32-bit compiler, while intrinsic
> functions are available in 32 and 64-bit compilers.
Why isn't it defined for the 64-bit compiler? (Because the config
test fails?) It seems that it should be defined regardless, wouldn't
you say? Even if we need to hardcode it _config.h or _defs.h...
Martin
>
>> 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*).
>
> Farid.
>
Re: [PATCH] _mutex.h
Posted by Farid Zaripov <Fa...@epam.com>.
From: Martin Sebor [mailto:sebor@roguewave.com]
Sent: Ср, 02.01.2008 20:41
To: stdcxx-dev@incubator.apache.org
Subject: Re: [PATCH] _mutex.h
>> +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?
No. _RWSTD_INTERLOCKED_T #defined only for 32-bit compiler, while intrinsic
functions are available in 32 and 64-bit compilers.
> 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*).
Farid.