public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: gthr-win32 issues with Windows 95
@ 2004-06-21 11:12 Wu Yongwei
  2004-06-23  2:52 ` Danny Smith
  0 siblings, 1 reply; 3+ messages in thread
From: Wu Yongwei @ 2004-06-21 11:12 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

My original patch improved the locking performance in Win32, but
problems occur with Windows 95, which does not support
InterlockedCompareExchange.  It is very serious in that the symbol is
undefined in libgcc.a, causing any new executables linked with it not
runnable on Windows 95.  Inline assembly is used to make it run on
Windows 95 (in fact, it is currently not used anywhere unless the user
manually call the ...trylock function; so it is a non-issue that it
requires i486).

Another issue is the weaker guarantee the Windows 95
InterlockedIncrement and InterlockedDecrement gives.  This can be fixed
by changing the default value and comparing the return value only with
0.

ChangeLog:

2004-05-27  Wu Yongwei  <adah@sh163.net>

	* gthr-win32.h (__GTHREAD_MUTEX_INIT_DEFAULT): Adjust.
	(__gthr_i486_lock_cmp_xchg): New inline assembly function.
	(__GTHR_W32_InterlockedCompareExchange): New macro to choose a
	suitable function for interlocked compare-and-exchange.
	(__gthread_mutex_trylock): Use
	__GTHR_W32_InterlockedCompareExchange.
	(__gthread_mutex_init_function, __gthread_mutex_lock,
	__gthread_mutex_trylock, __gthread_mutex_unlock): Adjust the
	initial counter value to work correctly under Windows 95.
	* config/i386/gthr-win32.c: Adjust include order.
	Define __GTHREAD_I486_INLINE_LOCK_PRIMITIVES before including
	gthr-win32.h.
	(__gthr_win32_mutex_init_function, __gthr_win32_mutex_lock,
	__gthr_win32_mutex_trylock, __gthr_win32_mutex_unlock): Adjust
	to match inline versions in gthr-win32.h.

Best regards,

Wu Yongwei

[-- Attachment #2: gthr-win32.diff --]
[-- Type: text/x-patch, Size: 5288 bytes --]

Index: gcc/gthr-win32.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/gthr-win32.h,v
retrieving revision 1.22
diff -u -p -r1.22 gthr-win32.h
--- gcc/gthr-win32.h	27 Apr 2004 21:38:05 -0000	1.22
+++ gcc/gthr-win32.h	27 May 2004 14:38:43 -0000
@@ -345,7 +345,7 @@ typedef struct {
 
 #define __GTHREAD_ONCE_INIT {0, -1}
 #define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function
-#define __GTHREAD_MUTEX_INIT_DEFAULT {0, 0}
+#define __GTHREAD_MUTEX_INIT_DEFAULT {-1, 0}
 
 #if __MINGW32_MAJOR_VERSION >= 1 || \
   (__MINGW32_MAJOR_VERSION == 0 && __MINGW32_MINOR_VERSION > 2)
@@ -357,6 +357,29 @@ extern int _CRT_MT;
 extern int __mingwthr_key_dtor (unsigned long, void (*) (void *));
 #endif /* __MINGW32__ version */
 
+#ifdef __GTHREAD_I486_INLINE_LOCK_PRIMITIVES
+
+static inline long
+__gthr_i486_lock_cmp_xchg(long *dest, long xchg, long comperand)
+{
+  long result;
+  __asm__ __volatile__ ("\n\
+	lock\n\
+	cmpxchg{l} {%4, %1|%1, %4}\n"
+	: "=a" (result), "=m" (*dest)
+	: "0" (comperand), "m" (*dest), "r" (xchg)
+	: "cc");
+  return result;
+}
+
+#define __GTHR_W32_InterlockedCompareExchange __gthr_i486_lock_cmp_xchg
+
+#else  /* __GTHREAD_I486_INLINE_LOCK_PRIMITIVES */
+
+#define __GTHR_W32_InterlockedCompareExchange InterlockedCompareExchange
+
+#endif /* __GTHREAD_I486_INLINE_LOCK_PRIMITIVES */
+
 static inline int
 __gthread_active_p (void)
 {
@@ -536,7 +559,7 @@ __gthread_setspecific (__gthread_key_t k
 static inline void
 __gthread_mutex_init_function (__gthread_mutex_t *mutex)
 {
-  mutex->counter = 0;
+  mutex->counter = -1;
   mutex->sema = CreateSemaphore (NULL, 0, 65535, NULL);
 }
 
@@ -547,13 +570,13 @@ __gthread_mutex_lock (__gthread_mutex_t 
 
   if (__gthread_active_p ())
     {
-      if (InterlockedIncrement (&mutex->counter) == 1 ||
+      if (InterlockedIncrement (&mutex->counter) == 0 ||
 	  WaitForSingleObject (mutex->sema, INFINITE) == WAIT_OBJECT_0)
 	status = 0;
       else
 	{
-	  // WaitForSingleObject returns WAIT_FAILED, and we can only do
-	  // some best-effort cleanup here.
+	  /* WaitForSingleObject returns WAIT_FAILED, and we can only do
+	     some best-effort cleanup here.  */
 	  InterlockedDecrement (&mutex->counter);
 	  status = 1;
 	}
@@ -568,7 +591,7 @@ __gthread_mutex_trylock (__gthread_mutex
 
   if (__gthread_active_p ())
     {
-      if (InterlockedCompareExchange (&mutex->counter, 1, 0 ) == 0)
+      if (__GTHR_W32_InterlockedCompareExchange (&mutex->counter, 0, -1) < 0)
 	status = 0;
       else
 	status = 1;
@@ -581,7 +604,7 @@ __gthread_mutex_unlock (__gthread_mutex_
 {
   if (__gthread_active_p ())
     {
-      if (InterlockedDecrement (&mutex->counter))
+      if (InterlockedDecrement (&mutex->counter) >= 0)
 	return ReleaseSemaphore (mutex->sema, 1, NULL) ? 0 : 1;
     }
   return 0;
Index: gcc/config/i386/gthr-win32.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/i386/gthr-win32.c,v
retrieving revision 1.6
diff -u -p -r1.6 gthr-win32.c
--- gcc/config/i386/gthr-win32.c	27 Apr 2004 21:38:05 -0000	1.6
+++ gcc/config/i386/gthr-win32.c	27 May 2004 14:38:43 -0000
@@ -31,11 +31,13 @@ Software Foundation, 59 Temple Place - S
    the executable file might be covered by the GNU General Public License.  */
 
 
+#include <windows.h>
 #ifndef __GTHREAD_HIDE_WIN32API
 # define __GTHREAD_HIDE_WIN32API 1
 #endif
+#undef  __GTHREAD_I486_INLINE_LOCK_PRIMITIVES
+#define __GTHREAD_I486_INLINE_LOCK_PRIMITIVES
 #include <gthr-win32.h>
-#include <windows.h>
 
 /* Windows32 threads specific definitions. The windows32 threading model
    does not map well into pthread-inspired gcc's threading model, and so 
@@ -144,20 +146,20 @@ __gthr_win32_setspecific (__gthread_key_
 void
 __gthr_win32_mutex_init_function (__gthread_mutex_t *mutex)
 {
-  mutex->counter = 0;
+  mutex->counter = -1;
   mutex->sema = CreateSemaphore (NULL, 0, 65535, NULL);
 }
 
 int
 __gthr_win32_mutex_lock (__gthread_mutex_t *mutex)
 {
-  if (InterlockedIncrement (&mutex->counter) == 1 ||
+  if (InterlockedIncrement (&mutex->counter) == 0 ||
       WaitForSingleObject (mutex->sema, INFINITE) == WAIT_OBJECT_0)
     return 0;
   else
     {
-      // WaitForSingleObject returns WAIT_FAILED, and we can only do
-      // some best-effort cleanup here.
+      /* WaitForSingleObject returns WAIT_FAILED, and we can only do
+         some best-effort cleanup here.  */
       InterlockedDecrement (&mutex->counter);
       return 1;
     }
@@ -166,7 +168,7 @@ __gthr_win32_mutex_lock (__gthread_mutex
 int
 __gthr_win32_mutex_trylock (__gthread_mutex_t *mutex)
 {
-  if (InterlockedCompareExchange (&mutex->counter, 1, 0 ) == 0)
+  if (__GTHR_W32_InterlockedCompareExchange (&mutex->counter, 0, -1) < 0)
     return 0;
   else
     return 1;
@@ -175,7 +177,7 @@ __gthr_win32_mutex_trylock (__gthread_mu
 int
 __gthr_win32_mutex_unlock (__gthread_mutex_t *mutex)
 {
-  if (InterlockedDecrement (&mutex->counter))
+  if (InterlockedDecrement (&mutex->counter) >= 0)
     return ReleaseSemaphore (mutex->sema, 1, NULL) ? 0 : 1;
   else
     return 0;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PATCH: gthr-win32 issues with Windows 95
  2004-06-21 11:12 PATCH: gthr-win32 issues with Windows 95 Wu Yongwei
@ 2004-06-23  2:52 ` Danny Smith
  2004-06-23  8:23   ` Christopher Faylor
  0 siblings, 1 reply; 3+ messages in thread
From: Danny Smith @ 2004-06-23  2:52 UTC (permalink / raw)
  To: GCC Patches, Wu Yongwei; +Cc: Christopher Faylor

From: "Wu Yongwei" | ChangeLog:
|
| 2004-05-27  Wu Yongwei  <adah@sh163.net>
|
| * gthr-win32.h (__GTHREAD_MUTEX_INIT_DEFAULT): Adjust.
| (__gthr_i486_lock_cmp_xchg): New inline assembly function.
| (__GTHR_W32_InterlockedCompareExchange): New macro to choose a
| suitable function for interlocked compare-and-exchange.
| (__gthread_mutex_trylock): Use
| __GTHR_W32_InterlockedCompareExchange.
| (__gthread_mutex_init_function, __gthread_mutex_lock,
| __gthread_mutex_trylock, __gthread_mutex_unlock): Adjust the
| initial counter value to work correctly under Windows 95.
| * config/i386/gthr-win32.c: Adjust include order.
| Define __GTHREAD_I486_INLINE_LOCK_PRIMITIVES before including
| gthr-win32.h.
| (__gthr_win32_mutex_init_function, __gthr_win32_mutex_lock,
| __gthr_win32_mutex_trylock, __gthr_win32_mutex_unlock): Adjust
| to match inline versions in gthr-win32.h.
|

This is fine by me but I think it could be improved by the addition of a
comment to gthr-win32.h, explaining why
__GTHREAD_I486_INLINE_LOCK_PRIMITIVES
 will have to be defined for apps targetting Win95.   Even though
mutex_try_lock (and hence __GTHR_W32_InterlockedCompareExchange) is not
currently used,  one day it might be.

Chris,  Is this okay with you for trunk.

Danny

| Best regards,
|
| Wu Yongwei
|



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PATCH: gthr-win32 issues with Windows 95
  2004-06-23  2:52 ` Danny Smith
@ 2004-06-23  8:23   ` Christopher Faylor
  0 siblings, 0 replies; 3+ messages in thread
From: Christopher Faylor @ 2004-06-23  8:23 UTC (permalink / raw)
  To: Danny Smith; +Cc: GCC Patches, Wu Yongwei

On Wed, Jun 23, 2004 at 01:43:19AM +0100, Danny Smith wrote:
>From: "Wu Yongwei" | ChangeLog:
>|
>| 2004-05-27  Wu Yongwei  <adah@sh163.net>
>|
>| * gthr-win32.h (__GTHREAD_MUTEX_INIT_DEFAULT): Adjust.
>| (__gthr_i486_lock_cmp_xchg): New inline assembly function.
>| (__GTHR_W32_InterlockedCompareExchange): New macro to choose a
>| suitable function for interlocked compare-and-exchange.
>| (__gthread_mutex_trylock): Use
>| __GTHR_W32_InterlockedCompareExchange.
>| (__gthread_mutex_init_function, __gthread_mutex_lock,
>| __gthread_mutex_trylock, __gthread_mutex_unlock): Adjust the
>| initial counter value to work correctly under Windows 95.
>| * config/i386/gthr-win32.c: Adjust include order.
>| Define __GTHREAD_I486_INLINE_LOCK_PRIMITIVES before including
>| gthr-win32.h.
>| (__gthr_win32_mutex_init_function, __gthr_win32_mutex_lock,
>| __gthr_win32_mutex_trylock, __gthr_win32_mutex_unlock): Adjust
>| to match inline versions in gthr-win32.h.
>|
>
>This is fine by me but I think it could be improved by the addition of a
>comment to gthr-win32.h, explaining why
>__GTHREAD_I486_INLINE_LOCK_PRIMITIVES
> will have to be defined for apps targetting Win95.   Even though
>mutex_try_lock (and hence __GTHR_W32_InterlockedCompareExchange) is not
>currently used,  one day it might be.
>
>Chris,  Is this okay with you for trunk.

Yes, thanks.

cgf

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2004-06-23  1:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-06-21 11:12 PATCH: gthr-win32 issues with Windows 95 Wu Yongwei
2004-06-23  2:52 ` Danny Smith
2004-06-23  8:23   ` Christopher Faylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).