On 19.12.2011 16:31, Robert Haas wrote:
On Sun, Dec 18, 2011 at 5:42 PM, Martin Pitt<mp...@debian.org>  wrote:
It probably makes sense to use it on any platform where it's
defined. Presumably an implementation provided by the compiler is
always going to be at least as good as any magic assembler
incantations we can come up with.

I agree. How about a patch like this? It uses builtin atomics if
available, and falls back to the custom implementations if not.

-1.  Absent some evidence that gcc's implementations are superior to
ours, I think we should not change stuff that works now.  That's
likely to lead to subtle bugs that are hard to find and perhaps
dependent on the exact compiler version used.

Ok, we're in disagreement on that then. I don't feel very strongly about it, let's see what others think.

One thing that caught my eye: if you use __sync_lock_and_test() to implement S_LOCK(), you really should be using __sync_lock_release() for S_UNLOCK().

Actually, I believe our Itanium (and possibly ARM, too) implementation of S_UNLOCK() is wrong as it is. There is no platform-specific S_UNLOCK() defined for Itanium, so we're using the generic implementation:

#if !defined(S_UNLOCK)
#define S_UNLOCK(lock)          (*((volatile slock_t *) (lock)) = 0)
#endif   /* S_UNLOCK */

That is not sufficient on platforms with a weak memory model, like Itanium.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to