From: Nathan Bossart <nathandbossart@gmail.com>
To: pgsql-hackers@postgresql.org
Subject: small cleanup for s_lock.h
Date: Mon, 4 May 2026 16:49:44 -0500
Message-ID: <afkUeI7UhacZ5ZFm@nathan> (raw)
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 <nathan@postgresql.org>
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.hindex 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 <nathan@postgresql.org>
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.hindex 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 pgsql-bugs@lists.postgresql.org.
#endif
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] v1-0001-Remove-fallback-declaration-for-tas.patch (1.0K, ../afkUeI7UhacZ5ZFm@nathan/2-v1-0001-Remove-fallback-declaration-for-tas.patch)
download | inline diff:
From a254cee054ccd22c6c11f876c5f8549f28f6499c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
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.hindex 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)
[text/plain] v1-0002-Remove-HAS_TEST_AND_SET.patch (3.2K, ../afkUeI7UhacZ5ZFm@nathan/3-v1-0002-Remove-HAS_TEST_AND_SET.patch)
download | inline diff:
From 60d6d767fc8e318f303c72b5883e3a15656b9ff5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
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.hindex 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 pgsql-bugs@lists.postgresql.org.
#endif
--
2.50.1 (Apple Git-155)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com
Subject: Re: small cleanup for s_lock.h
In-Reply-To: <afkUeI7UhacZ5ZFm@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox