Prefer int-wide pg_atomic_flag over char-wide when using gcc intrinsics.

tglsfdc · tglsfdc · commit 698127a4a9bc · 2017-01-04T13:36:55.000-05:00
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
diff --git 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;
 
