From 698127a4a9bc3c74659bf0e5383b1ed99aeb1570 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 4 Jan 2017 13:36:44 -0500 Subject: [PATCH] Prefer int-wide pg_atomic_flag over char-wide when using gcc intrinsics. configure can only probe the existence of gcc intrinsics, not how well they're implemented, and unfortunately the answer is sometimes "badly". In particular we've found that multiple compilers fail to implement char-width __sync_lock_test_and_set() correctly on PPC; and even a correct implementation would necessarily be pretty inefficient, since that hardware has only a word-wide primitive to work with. Given the knowledge we've accumulated in s_lock.h, it appears that it's best to rely on int-width TAS operations on most non-Intel architectures. Hence, pick int not char when both are nominally available to us in generic-gcc.h (note that that code is not used for x86[_64]). Back-patch to fix regression test failures on FreeBSD/PPC. Ordinarily back-patching a change like this would be verboten because of ABI breakage. But since pg_atomic_flag is not yet used in any Postgres data structure, there's no ABI to break. It seems safer to back-patch to avoid possible gotchas, if someday we do back-patch something that uses pg_atomic_flag. Discussion: https://postgr.es/m/25414.1483076673@sss.pgh.pa.us --- src/include/port/atomics/generic-gcc.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h index d4cfd31bd1..7efc0861e7 100644 --- a/src/include/port/atomics/generic-gcc.h +++ b/src/include/port/atomics/generic-gcc.h @@ -62,12 +62,15 @@ #define PG_HAVE_ATOMIC_FLAG_SUPPORT typedef struct pg_atomic_flag { - /* some platforms only have a 8 bit wide TAS */ -#ifdef HAVE_GCC__SYNC_CHAR_TAS - volatile char value; -#else - /* but an int works on more platforms */ + /* + * If we have a choice, use int-width TAS, because that is more efficient + * and/or more reliably implemented on most non-Intel platforms. (Note + * that this code isn't used on x86[_64]; see arch-x86.h for that.) + */ +#ifdef HAVE_GCC__SYNC_INT32_TAS volatile int value; +#else + volatile char value; #endif } pg_atomic_flag;