Hello Tom, all, happy new year everyone!
Tom Lane [2011-12-20 21:14 -0500]: > So I'm thinking that removing the swpb ASM option is not such a good > idea. We could possibly test for __sync_lock_test_and_set first, and > only use swpb if we're on ARM and don't have the builtin. Tom Lane [2011-12-21 10:55 -0500]: > Yeah, that was another thing I found worrisome while googling: there > were a disturbingly large number of claims that __sync_lock_test_and_set > and/or __sync_lock_release were flat-out broken on various combinations > of gcc version and platform. After reading that, there is no way at all > that I'd accept your original patch to use these functions everywhere. > > For the moment I'm inclined to consider using these functions *only* on > ARM, so as to limit our exposure to such bugs. That would also limit > the risk of using an inappropriate choice of lock width. OK, fair enough. New patch attached, which does exactly this now. Third time is the charm! Thanks, Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Description: Use gcc/intel cc builtin atomic operations for test-and-set on ARM, if available (http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html). The existing PostgreSQL implementation does not work on thumb2. Author: Martin Pitt <mp...@debian.org> Index: postgresql-9.1-9.1.2/configure.in =================================================================== --- postgresql-9.1-9.1.2.orig/configure.in 2011-12-01 22:47:20.000000000 +0100 +++ postgresql-9.1-9.1.2/configure.in 2012-01-07 10:10:51.153609202 +0100 @@ -1522,6 +1522,18 @@ AC_SUBST(LDAP_LIBS_FE) AC_SUBST(LDAP_LIBS_BE) +# gcc and intel compiler provide builtin functions for atomic test-and-set +AC_MSG_CHECKING([whether the C compiler provides atomic builtins]) +AC_TRY_LINK([], [int lock = 0; __sync_lock_test_and_set(&lock, 1); __sync_lock_release(&lock);], + [have_cc_atomics="yes"], + [have_cc_atomics="no"] +) +if test "$have_cc_atomics" = yes; then + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_CC_ATOMICS, 1, [Define to 1 if compiler provides atomic builtins]) +else + AC_MSG_RESULT(no) +fi # This test makes sure that run tests work at all. Sometimes a shared # library is found by the linker, but the runtime linker can't find it. Index: postgresql-9.1-9.1.2/configure =================================================================== --- postgresql-9.1-9.1.2.orig/configure 2011-12-01 22:47:20.000000000 +0100 +++ postgresql-9.1-9.1.2/configure 2012-01-07 10:10:51.177609204 +0100 @@ -23602,6 +23602,69 @@ +# gcc and intel compiler provide builtin functions for atomic test-and-set +{ $as_echo "$as_me:$LINENO: checking whether the C compiler provides atomic builtins" >&5 +$as_echo_n "checking whether the C compiler provides atomic builtins... " >&6; } +cat >conftest.$ac_ext <<_ACEOF +/* confdefs.h. */ +_ACEOF +cat confdefs.h >>conftest.$ac_ext +cat >>conftest.$ac_ext <<_ACEOF +/* end confdefs.h. */ + +int +main () +{ +int lock = 0; __sync_lock_test_and_set(&lock, 1); __sync_lock_release(&lock); + ; + return 0; +} +_ACEOF +rm -f conftest.$ac_objext conftest$ac_exeext +if { (ac_try="$ac_link" +case "(($ac_try" in + *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;; + *) ac_try_echo=$ac_try;; +esac +eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\"" +$as_echo "$ac_try_echo") >&5 + (eval "$ac_link") 2>conftest.er1 + ac_status=$? + grep -v '^ *+' conftest.er1 >conftest.err + rm -f conftest.er1 + cat conftest.err >&5 + $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); } && { + test -z "$ac_c_werror_flag" || + test ! -s conftest.err + } && test -s conftest$ac_exeext && { + test "$cross_compiling" = yes || + $as_test_x conftest$ac_exeext + }; then + have_cc_atomics="yes" +else + $as_echo "$as_me: failed program was:" >&5 +sed 's/^/| /' conftest.$ac_ext >&5 + + have_cc_atomics="no" + +fi + +rm -rf conftest.dSYM +rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \ + conftest$ac_exeext conftest.$ac_ext +if test "$have_cc_atomics" = yes; then + { $as_echo "$as_me:$LINENO: result: yes" >&5 +$as_echo "yes" >&6; } + +cat >>confdefs.h <<\_ACEOF +#define HAVE_CC_ATOMICS 1 +_ACEOF + +else + { $as_echo "$as_me:$LINENO: result: no" >&5 +$as_echo "no" >&6; } +fi # This test makes sure that run tests work at all. Sometimes a shared # library is found by the linker, but the runtime linker can't find it. Index: postgresql-9.1-9.1.2/src/include/pg_config.h.in =================================================================== --- postgresql-9.1-9.1.2.orig/src/include/pg_config.h.in 2011-12-01 22:47:20.000000000 +0100 +++ postgresql-9.1-9.1.2/src/include/pg_config.h.in 2012-01-07 10:10:51.657609229 +0100 @@ -87,6 +87,9 @@ /* Define to 1 if you have the `cbrt' function. */ #undef HAVE_CBRT +/* Define to 1 if compiler provides atomic builtins */ +#undef HAVE_CC_ATOMICS + /* Define to 1 if you have the `class' function. */ #undef HAVE_CLASS Index: postgresql-9.1-9.1.2/src/include/storage/s_lock.h =================================================================== --- postgresql-9.1-9.1.2.orig/src/include/storage/s_lock.h 2011-12-01 22:47:20.000000000 +0100 +++ postgresql-9.1-9.1.2/src/include/storage/s_lock.h 2012-01-07 10:15:38.285623102 +0100 @@ -255,10 +255,25 @@ #if defined(__arm__) || defined(__arm) #define HAS_TEST_AND_SET -typedef unsigned char slock_t; - #define TAS(lock) tas(lock) +#ifdef HAVE_CC_ATOMICS +typedef int slock_t; /* Note: must use int; char fails on some boards */ + +/* Use compiler provided atomic builtins (gcc/Intel), which work on current ARM + * cores/thumb2 */ +static __inline__ int +tas(volatile slock_t *lock) +{ + return __sync_lock_test_and_set (lock, 1); +} + +#define S_UNLOCK(lock) __sync_lock_release(lock) + +#else +typedef unsigned char slock_t; + +/* Note: does not work with thumb2 */ static __inline__ int tas(volatile slock_t *lock) { @@ -271,6 +286,7 @@ : "memory"); return (int) _res; } +#endif #endif /* __arm__ */
signature.asc
Description: Digital signature
