Author: Noah Misch Commit: Noah Misch 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;