Martin Pitt <mp...@debian.org> writes: > The updated patch only uses the gcc builtins if there is no explicit > implementation, but drops the arm one as this doesn't work on ARMv7 > and newer, as stated in the original mail.
Getting this thread back to the original patch ... I'm afraid that if we apply this as-is, what will happen is that we fix ARMv7 and break older versions. Some googling found this, for instance: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33413 which suggests that (1) ARM gcc hasn't had __sync_lock_test_and_set for very long, and (2) what it generates doesn't work pre-ARMv6. 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. Another thing that is bothering me is that according to the gcc manual, eg here, http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html __sync_lock_test_and_set is nominally provided for datatypes 1, 2, 4, or 8 bytes in length, but the underlying hardware doesn't necessarily support all those widths natively. If you pick the wrong width then you don't get an inline operation at all, but a call to some possibly inefficient library subroutine. I see that your patch just assumes that "int" will be a good width for the lock type, but it's unclear to me what that choice is based on and whether or not it might be a really bad choice on some platforms. A look through s_lock.h suggests that only a minority of platforms prefer int-width locks ... but I have no idea how many of those assembly snippets could have been coded to use a different lock datatype without penalty. Some other evidence that 4-byte __sync_lock_test_and_set isn't universal is here: https://svn.boost.org/trac/boost/ticket/2525 Google is also finding some rather worrisome suggestions that __sync_lock_test_and_set might involve a kernel call on some flavors of ARM. That would be pretty disastrous from a performance standpoint. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
