compare_exchange-ppc-immediate-v1.patch

text/plain

Patch

Format: unified
Series: patch v1
File+
src/include/port/atomics/arch-ppc.h 4 2
src/test/regress/regress.c 8 0
Author:     Noah Misch <noah@leadboat.com>
Commit:     Noah Misch <noah@leadboat.com>

    Choose ppc compare_exchange constant path for more operand values.
    
    The implementation uses smaller code when the "expected" operand is a
    small constant, but the implementation needlessly defined the set of
    acceptable constants more narrowly than the ABI does.  Core PostgreSQL
    and PGXN don't use the constant path at all, so this is future-proofing.
    
    Reviewed by FIXME.  Reported by Christoph Berg.
    
    Discussion: https://postgr.es/m/20201009092825.GD889580@msg.df7cb.de

diff --git a/src/include/port/atomics/arch-ppc.h b/src/include/port/atomics/arch-ppc.h
index 68e6603..9b2e499 100644
--- a/src/include/port/atomics/arch-ppc.h
+++ b/src/include/port/atomics/arch-ppc.h
@@ -94,7 +94,8 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
 
 #ifdef HAVE_I_CONSTRAINT__BUILTIN_CONSTANT_P
 	if (__builtin_constant_p(*expected) &&
-		*expected <= PG_INT16_MAX && *expected >= PG_INT16_MIN)
+		(int32) *expected <= PG_INT16_MAX &&
+		(int32) *expected >= PG_INT16_MIN)
 		__asm__ __volatile__(
 			"	sync				\n"
 			"	lwarx   %0,0,%5		\n"
@@ -183,7 +184,8 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
 	/* Like u32, but s/lwarx/ldarx/; s/stwcx/stdcx/; s/cmpw/cmpd/ */
 #ifdef HAVE_I_CONSTRAINT__BUILTIN_CONSTANT_P
 	if (__builtin_constant_p(*expected) &&
-		*expected <= PG_INT16_MAX && *expected >= PG_INT16_MIN)
+		(int64) *expected <= PG_INT16_MAX &&
+		(int64) *expected >= PG_INT16_MIN)
 		__asm__ __volatile__(
 			"	sync				\n"
 			"	ldarx   %0,0,%5		\n"
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 02397f2..09bc42a 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -720,6 +720,14 @@ test_atomic_uint32(void)
 	EXPECT_EQ_U32(pg_atomic_read_u32(&var), (uint32) INT_MAX + 1);
 	EXPECT_EQ_U32(pg_atomic_sub_fetch_u32(&var, INT_MAX), 1);
 	pg_atomic_sub_fetch_u32(&var, 1);
+	expected = PG_INT16_MAX;
+	EXPECT_TRUE(!pg_atomic_compare_exchange_u32(&var, &expected, 1));
+	expected = PG_INT16_MAX + 1;
+	EXPECT_TRUE(!pg_atomic_compare_exchange_u32(&var, &expected, 1));
+	expected = PG_INT16_MIN;
+	EXPECT_TRUE(!pg_atomic_compare_exchange_u32(&var, &expected, 1));
+	expected = PG_INT16_MIN - 1;
+	EXPECT_TRUE(!pg_atomic_compare_exchange_u32(&var, &expected, 1));
 
 	/* fail exchange because of old expected */
 	expected = 10;