I noticed that s_lock.h points to a default implementation of tas() in tas.s or s_lock.c, but AFAICT there hasn't been a tas() implementation in s_lock.c since commit 718aa43a4e, and commit 25f36066dd seems to have removed the last remaining tas.s files. So, I think this is dead code.
I also noticed that HAS_TEST_AND_SET just means that TAS is defined, so I wrote a 0002 that removes it in favor of checking TAS directly. I'd like to rewrite the comment at the top of the file, too, but haven't gotten to that yet. I find it a little misleading, especially because we #error if TAS isn't defined. -- nathan
>From a254cee054ccd22c6c11f876c5f8549f28f6499c Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Mon, 4 May 2026 16:04:04 -0500 Subject: [PATCH v1 1/2] Remove fallback declaration for tas(). The last definition of tas() in s_lock.c was removed in commit 718aa43a4e, and the last tas.s file was removed in commit 25f36066dd, so this is dead code. --- src/include/storage/s_lock.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index c9e52511990..dcfec8ce2af 100644 --- a/src/include/storage/s_lock.h +++ b/src/include/storage/s_lock.h @@ -697,13 +697,6 @@ extern void s_unlock(volatile slock_t *lock); #define SPIN_DELAY() ((void) 0) #endif /* SPIN_DELAY */ -#if !defined(TAS) -extern int tas(volatile slock_t *lock); /* in port/.../tas.s, or - * s_lock.c */ - -#define TAS(lock) tas(lock) -#endif /* TAS */ - #if !defined(TAS_SPIN) #define TAS_SPIN(lock) TAS(lock) #endif /* TAS_SPIN */ -- 2.50.1 (Apple Git-155)
>From 60d6d767fc8e318f303c72b5883e3a15656b9ff5 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Mon, 4 May 2026 16:24:30 -0500 Subject: [PATCH v1 2/2] Remove HAS_TEST_AND_SET. This is only set when TAS is defined, so we can just check whether TAS is defined directly instead. --- src/include/storage/s_lock.h | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index dcfec8ce2af..fb872edd2f0 100644 --- a/src/include/storage/s_lock.h +++ b/src/include/storage/s_lock.h @@ -124,7 +124,6 @@ #ifdef __i386__ /* 32-bit i386 */ -#define HAS_TEST_AND_SET typedef unsigned char slock_t; @@ -194,7 +193,6 @@ spin_delay(void) #ifdef __x86_64__ /* AMD Opteron, Intel EM64T */ -#define HAS_TEST_AND_SET typedef unsigned char slock_t; @@ -249,7 +247,6 @@ spin_delay(void) */ #if defined(__arm__) || defined(__arm) || defined(__aarch64__) #ifdef HAVE_GCC__SYNC_INT32_TAS -#define HAS_TEST_AND_SET #define TAS(lock) tas(lock) @@ -292,7 +289,6 @@ spin_delay(void) /* S/390 and S/390x Linux (32- and 64-bit zSeries) */ #if defined(__s390__) || defined(__s390x__) -#define HAS_TEST_AND_SET typedef unsigned int slock_t; @@ -321,7 +317,6 @@ tas(volatile slock_t *lock) * acquire/release semantics. The CPU will treat superfluous members as * NOPs, so it's just code space. */ -#define HAS_TEST_AND_SET typedef unsigned char slock_t; @@ -392,7 +387,6 @@ do \ /* PowerPC */ #if defined(__ppc__) || defined(__powerpc__) || defined(__ppc64__) || defined(__powerpc64__) -#define HAS_TEST_AND_SET typedef unsigned int slock_t; @@ -453,7 +447,6 @@ do \ #if defined(__mips__) && !defined(__sgi) /* non-SGI MIPS */ -#define HAS_TEST_AND_SET typedef unsigned int slock_t; @@ -531,10 +524,9 @@ do \ * grounds that that's known to be more likely to work in the ARM ecosystem. * (But we dealt with ARM above.) */ -#if !defined(HAS_TEST_AND_SET) +#if !defined(TAS) #if defined(HAVE_GCC__SYNC_INT32_TAS) -#define HAS_TEST_AND_SET #define TAS(lock) tas(lock) @@ -549,7 +541,6 @@ tas(volatile slock_t *lock) #define S_UNLOCK(lock) __sync_lock_release(lock) #elif defined(HAVE_GCC__SYNC_CHAR_TAS) -#define HAS_TEST_AND_SET #define TAS(lock) tas(lock) @@ -565,7 +556,7 @@ tas(volatile slock_t *lock) #endif /* HAVE_GCC__SYNC_INT32_TAS */ -#endif /* !defined(HAS_TEST_AND_SET) */ +#endif /* !defined(TAS) */ /* @@ -592,12 +583,11 @@ tas(volatile slock_t *lock) * --------------------------------------------------------------------- */ -#if !defined(HAS_TEST_AND_SET) /* We didn't trigger above, let's try here */ +#if !defined(TAS) /* We didn't trigger above, let's try here */ #ifdef _MSC_VER typedef LONG slock_t; -#define HAS_TEST_AND_SET #define TAS(lock) (InterlockedCompareExchange(lock, 1, 0)) #define SPIN_DELAY() spin_delay() @@ -649,11 +639,11 @@ spin_delay(void) #endif -#endif /* !defined(HAS_TEST_AND_SET) */ +#endif /* !defined(TAS) */ /* Blow up if we didn't have any way to do spinlocks */ -#ifndef HAS_TEST_AND_SET +#ifndef TAS #error PostgreSQL does not have spinlock support on this platform. Please report this to [email protected]. #endif -- 2.50.1 (Apple Git-155)
