diff mbox

[trans-mem] __sync_add_and_fetch_8 on ia32

Message ID 20100707155108.GA20458@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez July 7, 2010, 3:51 p.m. UTC 
> > ... a slightly more sophisticated variant of b) would be using
> > uint64_t for 64-bit targets and uint32_t for 32-bit targets, should
> > make everybody happy.
> 
> There's a correctness issue in the library interface -- there's no
> clean way to handle that value overflowing.  Rather than make the
> library interface way more complex, it seemed far easier to simply
> use a 64-bit value, which we simply assume won't overflow.
> 
> I'd be fine with -m586 (which is how things will get built on e.g.
> Fedora), but that isn't going to help other 32-bit targets that miss
> that operation.  We'll need a configure test and a mutex if needed.

Sorry to have dropped the ball on this for so long, but 32-bit problems
weren't too high on my priority list :-).

I have added configury magic to check for 64-bit sync builtins.  If
there are no 64-bit builtins, we implement the global thread id with
POSIX threads.

Alas, this surfaced some seemingly unrelated runtime problems in 32-bit
land, which I'll tackle next.

At least we can now build and run (some) tests.  Yay!

Tested on x86-64 with: RUNTESTFLAGS="--target_board=unix'{-m32,-m64}'"

OK for branch?

	* configure.ac: Call LIBITM_CHECK_64BIT_SYNC_BUILTINS.
	* beginend.cc (begin_transaction): If 64-bit sync builtins are not
	available, use pthread mutexes.
	* acinclude.m4 (LIBITM_CHECK_64BIT_SYNC_BUILTINS): New.
	* config.h.in: Regenerate.
	* configure: Regenerate.

Comments

Richard Henderson July 7, 2010, 5:10 p.m. UTC | #1 
On 07/07/2010 08:51 AM, Aldy Hernandez wrote:
> 	* configure.ac: Call LIBITM_CHECK_64BIT_SYNC_BUILTINS.
> 	* beginend.cc (begin_transaction): If 64-bit sync builtins are not
> 	available, use pthread mutexes.
> 	* acinclude.m4 (LIBITM_CHECK_64BIT_SYNC_BUILTINS): New.
> 	* config.h.in: Regenerate.
> 	* configure: Regenerate.

Ok.


r~
diff mbox

Patch

Index: configure.ac
===================================================================
--- configure.ac	(revision 161318)
+++ configure.ac	(working copy)
@@ -216,6 +216,7 @@  CFLAGS="$save_CFLAGS $XCFLAGS"
 # Check for __sync_val_compare_and_swap, but only after the target has
 # had a chance to set XCFLAGS.
 LIBITM_CHECK_SYNC_BUILTINS
+LIBITM_CHECK_64BIT_SYNC_BUILTINS
 
 # Cleanup and exit.
 CFLAGS="$save_CFLAGS"
Index: beginend.cc
===================================================================
--- beginend.cc	(revision 161318)
+++ beginend.cc	(working copy)
@@ -84,6 +84,10 @@  GTM::gtm_transaction::operator delete(vo
   thr->free_tx[idx] = tx;
 }
 
+#ifndef HAVE_64BIT_SYNC_BUILTINS
+static pthread_mutex_t global_tid_lock = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 uint32_t
 GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 {
@@ -99,7 +103,13 @@  GTM::gtm_transaction::begin_transaction 
   tx->prev = gtm_tx();
   if (tx->prev)
     tx->nesting = tx->prev->nesting + 1;
+#ifdef HAVE_64BIT_SYNC_BUILTINS
   tx->id = __sync_add_and_fetch (&global_tid, 1);
+#else
+  pthread_mutex_lock (&global_tid_lock);
+  tx->id = ++global_tid;
+  pthread_mutex_unlock (&global_tid_lock);
+#endif
   tx->jb = *jb;
 
   set_gtm_tx (tx);
Index: acinclude.m4
===================================================================
--- acinclude.m4	(revision 161318)
+++ acinclude.m4	(working copy)
@@ -12,6 +12,20 @@  AC_DEFUN([LIBITM_CHECK_SYNC_BUILTINS], [
 	      [Define to 1 if the target supports __sync_*_compare_and_swap])
   fi])
 
+dnl Check whether the target supports 64-bit __sync_*_compare_and_swap.
+AC_DEFUN([LIBITM_CHECK_64BIT_SYNC_BUILTINS], [
+  AC_CACHE_CHECK([whether the target supports 64-bit __sync_*_compare_and_swap],
+		 libitm_cv_have_64bit_sync_builtins, [
+  AC_TRY_LINK([#include <stdint.h>],
+    [uint64_t foo, bar;
+     bar = __sync_val_compare_and_swap(&foo, 0, 1);],
+    libitm_cv_have_64bit_sync_builtins=yes,
+    libitm_cv_have_64bit_sync_builtins=no)])
+    if test $libitm_cv_have_64bit_sync_builtins = yes; then
+      AC_DEFINE(HAVE_64BIT_SYNC_BUILTINS, 1,
+	        [Define to 1 if the target supports 64-bit __sync_*_compare_and_swap])
+  fi])
+
 dnl Check whether the target supports hidden visibility.
 AC_DEFUN([LIBITM_CHECK_ATTRIBUTE_VISIBILITY], [
   AC_CACHE_CHECK([whether the target supports hidden visibility],