From 6e5de703b6c791d355936a61abb52b3b1fc6e184 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 a3abc8a1a9..4e16098739 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;