Re: [HACKERS] xlc atomics
On Mon, Apr 25, 2016 at 11:52:04AM -0700, Andres Freund wrote: > On 2016-04-23 21:54:07 -0400, Noah Misch wrote: > > The bug is that the pg_atomic_compare_exchange_*() specifications > > grant "full barrier semantics", but generic-xlc.h provided only the > > semantics of an acquire barrier. > > I find the docs at > http://www-01.ibm.com/support/knowledgecenter/SSGH3R_13.1.2/com.ibm.xlcpp131.aix.doc/compiler_ref/bifs_sync_atomic.html > to be be awfully silent about that matter. I guess you just looked at > the assembler code? It's nice that one can figure out stuff like that > from an architecture manual, but it's sad that the docs for the > intrinsics is silent about that matter. Right. The intrinsics provide little value as abstractions if one checks the generated code to deduce how to use them. I was tempted to replace the intrinsics calls with inline asm. At least these functions are unlikely to change over time. > Except that I didn't verify the rs6000_pre_atomic_barrier() and > __fetch_and_add() internals about emitted sync/isync, the patch looks > good. We've so far not referred to "sequential consistency", but given > it's rise in popularity, I don't think it hurts. Thanks; committed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On 2016-04-23 21:54:07 -0400, Noah Misch wrote: > I missed a second synchronization bug in generic-xlc.h, but the pgbench test > suite caught it promptly after commit 008608b. Nice catch. > The bug is that the pg_atomic_compare_exchange_*() specifications > grant "full barrier semantics", but generic-xlc.h provided only the > semantics of an acquire barrier. I find the docs at http://www-01.ibm.com/support/knowledgecenter/SSGH3R_13.1.2/com.ibm.xlcpp131.aix.doc/compiler_ref/bifs_sync_atomic.html to be be awfully silent about that matter. I guess you just looked at the assembler code? It's nice that one can figure out stuff like that from an architecture manual, but it's sad that the docs for the intrinsics is silent about that matter. I do wonder if I shouldn't have left xlc fall by the wayside, and make it use the fallback implementation. Given it's popularity (or rather lack thereof) that'd probably have been a better choice. But that ship has sailed. Except that I didn't verify the rs6000_pre_atomic_barrier() and __fetch_and_add() internals about emitted sync/isync, the patch looks good. We've so far not referred to "sequential consistency", but given it's rise in popularity, I don't think it hurts. Stricly speaking generic-xlc.c shouldn't contain inline-asm, but given xlc is only there for power... Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On Mon, Feb 15, 2016 at 02:02:41PM -0500, Tom Lane wrote:
> Noah Misch writes:
> > That commit (0d32d2e) permitted things to compile and usually pass tests,
> > but
> > I missed the synchronization bug. Since 2015-10-01, the buildfarm has seen
> > sixteen duplicate-catalog-OID failures.
>
> I'd been wondering about those ...
>
> > These suggested OidGenLock wasn't doing its job. I've seen similar symptoms
> > around WALInsertLocks with "IBM XL C/C++ for Linux, V13.1.2 (5725-C73,
> > 5765-J08)" for ppc64le. The problem is generic-xlc.h
> > pg_atomic_compare_exchange_u32_impl() issuing __isync() before
> > __compare_and_swap(). __isync() shall follow __compare_and_swap(); see our
> > own s_lock.h, its references, and other projects' usage:
>
> Nice catch!
>
> > This patch's test case would have failed about half the time under today's
> > generic-xlc.h. Fast machines run it in about 1s. A test that detects the
> > bug
> > 99% of the time would run far longer, hence this compromise.
>
> Sounds like a reasonable compromise to me, although I wonder about the
> value of it if we stick it into pgbench's TAP tests. How many of the
> slower buildfarm members are running the TAP tests? Certainly mine are
> not.
I missed a second synchronization bug in generic-xlc.h, but the pgbench test
suite caught it promptly after commit 008608b. Buildfarm members mandrill and
hornet failed[1] or deadlocked within two runs. The bug is that the
pg_atomic_compare_exchange_*() specifications grant "full barrier semantics",
but generic-xlc.h provided only the semantics of an acquire barrier. Commit
008608b exposed that older problem; its LWLockWaitListUnlock() relies on
pg_atomic_compare_exchange_u32() (via pg_atomic_fetch_and_u32()) for release
barrier semantics.
The fix is to issue "sync" before each compare-and-swap, in addition to the
"isync" after it. This is consistent with the corresponding GCC atomics. The
pgbench test suite survived dozens of runs so patched.
[1]
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mandrill&dt=2016-04-11%2003%3A14%3A13
diff --git a/src/include/port/atomics/generic-xlc.h
b/src/include/port/atomics/generic-xlc.h
index f24e3af..f4fd2f3 100644
--- a/src/include/port/atomics/generic-xlc.h
+++ b/src/include/port/atomics/generic-xlc.h
@@ -40,12 +40,22 @@ static inline bool
pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
uint32
*expected, uint32 newval)
{
+ boolret;
+
+ /*
+* atomics.h specifies sequential consistency ("full barrier semantics")
+* for this interface. Since "lwsync" provides acquire/release
+* consistency only, do not use it here. GCC atomics observe the same
+* restriction; see its rs6000_pre_atomic_barrier().
+*/
+ __asm__ __volatile__ (" sync \n" ::: "memory");
+
/*
* XXX: __compare_and_swap is defined to take signed parameters, but
that
* shouldn't matter since we don't perform any arithmetic operations.
*/
- boolret = __compare_and_swap((volatile int*)&ptr->value,
-
(int *)expected, (int)newval);
+ ret = __compare_and_swap((volatile int*)&ptr->value,
+(int *)expected,
(int)newval);
/*
* xlc's documentation tells us:
@@ -63,6 +73,10 @@ pg_atomic_compare_exchange_u32_impl(volatile
pg_atomic_uint32 *ptr,
static inline uint32
pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
{
+ /*
+* __fetch_and_add() emits a leading "sync" and trailing "isync",
thereby
+* providing sequential consistency. This is undocumented.
+*/
return __fetch_and_add((volatile int *)&ptr->value, add_);
}
@@ -73,8 +87,12 @@ static inline bool
pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
uint64
*expected, uint64 newval)
{
- boolret = __compare_and_swaplp((volatile long*)&ptr->value,
-
(long *)expected, (long)newval);
+ boolret;
+
+ __asm__ __volatile__ (" sync \n" ::: "memory");
+
+ ret = __compare_and_swaplp((volatile long*)&ptr->value,
+ (long *)expected,
(long)newval);
__isync();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On 5 July 2015 at 06:54, Andres Freund wrote: > I wrote this entirely blindly, as evidenced here by the changes you > needed. At the time somebody had promised to soon put up an aix animal, > but that apparently didn't work out. > Similarly, I asked IBM for XL/C for a POWER7 Linux VM to improve test cover on the build farm but was told to just use gcc. It was with regards to testing hardware decfloat support and the folks I spoke to said that gcc's support was derived from that in xl/c anyway. They're not desperately keen to get their products out there for testing. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] xlc atomics
Noah Misch writes: > That commit (0d32d2e) permitted things to compile and usually pass tests, but > I missed the synchronization bug. Since 2015-10-01, the buildfarm has seen > sixteen duplicate-catalog-OID failures. I'd been wondering about those ... > These suggested OidGenLock wasn't doing its job. I've seen similar symptoms > around WALInsertLocks with "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, > 5765-J08)" for ppc64le. The problem is generic-xlc.h > pg_atomic_compare_exchange_u32_impl() issuing __isync() before > __compare_and_swap(). __isync() shall follow __compare_and_swap(); see our > own s_lock.h, its references, and other projects' usage: Nice catch! > This patch's test case would have failed about half the time under today's > generic-xlc.h. Fast machines run it in about 1s. A test that detects the bug > 99% of the time would run far longer, hence this compromise. Sounds like a reasonable compromise to me, although I wonder about the value of it if we stick it into pgbench's TAP tests. How many of the slower buildfarm members are running the TAP tests? Certainly mine are not. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On Mon, Feb 15, 2016 at 06:33:42PM +0100, Andres Freund wrote: > On 2016-02-15 12:11:29 -0500, Noah Misch wrote: > > These suggested OidGenLock wasn't doing its job. I've seen similar symptoms > > around WALInsertLocks with "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, > > 5765-J08)" for ppc64le. The problem is generic-xlc.h > > pg_atomic_compare_exchange_u32_impl() issuing __isync() before > > __compare_and_swap(). __isync() shall follow __compare_and_swap(); see our > > own s_lock.h, its references, and other projects' usage: > > Ugh. You're right! It's about not moving code before the stwcx... > > Do you want to add the new test (no objection, curious), or is that more > for testing? The patch's test would join PostgreSQL indefinitely. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On 2016-02-15 12:11:29 -0500, Noah Misch wrote: > These suggested OidGenLock wasn't doing its job. I've seen similar symptoms > around WALInsertLocks with "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, > 5765-J08)" for ppc64le. The problem is generic-xlc.h > pg_atomic_compare_exchange_u32_impl() issuing __isync() before > __compare_and_swap(). __isync() shall follow __compare_and_swap(); see our > own s_lock.h, its references, and other projects' usage: Ugh. You're right! It's about not moving code before the stwcx... Do you want to add the new test (no objection, curious), or is that more for testing? Greetings, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On Sat, Jul 04, 2015 at 08:07:49PM -0400, Noah Misch wrote:
> On Sun, Jul 05, 2015 at 12:54:43AM +0200, Andres Freund wrote:
> > On 2015-07-04 18:40:41 -0400, Noah Misch wrote:
> > > (1) "IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)". Getting it
> > > working
> > > required the attached patch.
>
> > Will you apply? Having the ability to test change seems to put you in a
> > much better spot then me.
>
> I will.
That commit (0d32d2e) permitted things to compile and usually pass tests, but
I missed the synchronization bug. Since 2015-10-01, the buildfarm has seen
sixteen duplicate-catalog-OID failures. The compiler has always been xlC, and
the branch has been HEAD or 9.5. Examples:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2015-12-15%2004%3A12%3A49
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2015-12-08%2001%3A20%3A16
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mandrill&dt=2015-12-11%2005%3A25%3A54
These suggested OidGenLock wasn't doing its job. I've seen similar symptoms
around WALInsertLocks with "IBM XL C/C++ for Linux, V13.1.2 (5725-C73,
5765-J08)" for ppc64le. The problem is generic-xlc.h
pg_atomic_compare_exchange_u32_impl() issuing __isync() before
__compare_and_swap(). __isync() shall follow __compare_and_swap(); see our
own s_lock.h, its references, and other projects' usage:
https://github.com/boostorg/smart_ptr/blob/boost-1.60.0/include/boost/smart_ptr/detail/sp_counted_base_vacpp_ppc.hpp#L55
http://redmine.gromacs.org/projects/gromacs/repository/entry/src/external/thread_mpi/include/thread_mpi/atomic/xlc_ppc.h?rev=v5.1.2#L112
https://www-01.ibm.com/support/knowledgecenter/SSGH2K_13.1.2/com.ibm.xlc131.aix.doc/language_ref/asm_example.html
This patch's test case would have failed about half the time under today's
generic-xlc.h. Fast machines run it in about 1s. A test that detects the bug
99% of the time would run far longer, hence this compromise.
Archaeology enthusiasts may enjoy the bug's changing presentation over time.
LWLockAcquire() exposed it after commit ab5194e redesigned lwlock.c in terms
of atomics. Assert builds were unaffected for commits in [ab5194e, bbfd7ed);
atomics.h asserts fattened code enough to mask the bug. However, removing the
AssertPointerAlignment() from pg_atomic_fetch_or_u32() would have sufficed to
surface it in assert builds. All builds demonstrated the bug once bbfd7ed
introduced xlC pg_attribute_noreturn(), which elicits a simpler instruction
sequence around the ExceptionalCondition() call embedded in each Assert.
nm
diff --git a/src/bin/pgbench/.gitignore b/src/bin/pgbench/.gitignore
index aae819e..983a3cd 100644
--- a/src/bin/pgbench/.gitignore
+++ b/src/bin/pgbench/.gitignore
@@ -1,3 +1,4 @@
/exprparse.c
/exprscan.c
/pgbench
+/tmp_check/
diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index 18fdf58..e9b1b74 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -40,3 +40,9 @@ clean distclean:
maintainer-clean: distclean
rm -f exprparse.c exprscan.c
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
diff --git a/src/bin/pgbench/t/001_pgbench.pl b/src/bin/pgbench/t/001_pgbench.pl
new file mode 100644
index 000..88e83ab
--- /dev/null
+++ b/src/bin/pgbench/t/001_pgbench.pl
@@ -0,0 +1,25 @@
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 3;
+
+# Test concurrent insertion into table with UNIQUE oid column. DDL expects
+# GetNewOidWithIndex() to successfully avoid violating uniqueness for indexes
+# like pg_class_oid_index and pg_proc_oid_index. This indirectly exercises
+# LWLock and spinlock concurrency. This test makes a 5-MiB table.
+my $node = get_new_node('main');
+$node->init;
+$node->start;
+$node->psql('postgres',
+ 'CREATE UNLOGGED TABLE oid_tbl () WITH OIDS; '
+ . 'ALTER TABLE oid_tbl ADD UNIQUE (oid);');
+my $script = $node->basedir . '/pgbench_script';
+append_to_file($script,
+ 'INSERT INTO oid_tbl SELECT FROM generate_series(1,1000);');
+$node->command_like(
+ [ qw(pgbench --no-vacuum --client=5 --protocol=prepared
+ --transactions=25 --file), $script ],
+ qr{processed: 125/125},
+ 'concurrent OID generation');
diff --git a/src/include/port/atomics/generic-xlc.h
b/src/include/port/atomics/generic-xlc.h
index 1e8a11e..f24e3af 100644
--- a/src/include/port/atomics/generic-xlc.h
+++ b/src/include/port/atomics/generic-xlc.h
@@ -41,18 +41,22 @@ pg_atomic_compare_exchange_u32_impl(volatile
pg_atomic_uint32 *ptr,
uint32
*expected, uint32 newval)
{
/*
+* XXX: __compare_and_swap is defined to take signed parameters, but
that
+* shouldn't matter since we don't perform any arithmetic operations.
+*/
+ boolret = __compare_and_swap((volatile int*)&ptr->value,
+
Re: [HACKERS] xlc atomics
On Sun, Jul 05, 2015 at 12:54:43AM +0200, Andres Freund wrote: > On 2015-07-04 18:40:41 -0400, Noah Misch wrote: > > (1) "IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)". Getting it working > > required the attached patch. > Will you apply? Having the ability to test change seems to put you in a > much better spot then me. I will. > > (2) "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, 5765-J08)" for ppc64le, > > http://www-01.ibm.com/support/docview.wss?uid=swg27044056&aid=1. This > > compiler has a Clang-derived C frontend. It defines __GNUC__ and offers > > GCC-style __sync_* atomics. > > Phew. I don't see much reason to try to support this. Why would that be > interesting? > > > Therefore, PostgreSQL selects generic-gcc.h. > > test_atomic_ops() fails because __sync_lock_test_and_set() of one-byte types > > segfaults at runtime. I have reported this to the vendor. Adding > > "pgac_cv_gcc_sync_char_tas=no" to the "configure" invocation is a good > > workaround. I could add a comment about that to > > src/test/regress/sql/lock.sql > > for affected folks to see in regression.diffs. To do better, we could make > > PGAC_HAVE_GCC__SYNC_CHAR_TAS perform a runtime test where possible. Yet > > another option is to force use of generic-xlc.h on this compiler. > > It seems fair enough to simply add another test and include > generic-xlc.h in that case. If it's indeed xlc, why not? Works for me. I'll do that as attached. diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h index 1a4c748..97a0064 100644 --- a/src/include/port/atomics.h +++ b/src/include/port/atomics.h @@ -81,8 +81,15 @@ * * pg_atomic_compare_exchange_u32(), pg_atomic_fetch_add_u32() * using compiler intrinsics are a good idea. */ +/* + * Given a gcc-compatible xlc compiler, prefer the xlc implementation. The + * ppc64le "IBM XL C/C++ for Linux, V13.1.2" implements both interfaces, but + * __sync_lock_test_and_set() of one-byte types elicits SIGSEGV. + */ +#if defined(__IBMC__) || defined(__IBMCPP__) +#include "port/atomics/generic-xlc.h" /* gcc or compatible, including clang and icc */ -#if defined(__GNUC__) || defined(__INTEL_COMPILER) +#elif defined(__GNUC__) || defined(__INTEL_COMPILER) #include "port/atomics/generic-gcc.h" #elif defined(WIN32_ONLY_COMPILER) #include "port/atomics/generic-msvc.h" @@ -90,8 +97,6 @@ #include "port/atomics/generic-acc.h" #elif defined(__SUNPRO_C) && !defined(__GNUC__) #include "port/atomics/generic-sunpro.h" -#elif (defined(__IBMC__) || defined(__IBMCPP__)) && !defined(__GNUC__) -#include "port/atomics/generic-xlc.h" #else /* * Unsupported compiler, we'll likely use slower fallbacks... At least -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlc atomics
On 2015-07-04 18:40:41 -0400, Noah Misch wrote: > (1) "IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)". Getting it working > required the attached patch. None of my xlc configurations have an > header, and a web search turned up no evidence of one in connection with xlc > platforms. Did you learn of a configuration needing atomic.h under xlc? The > rest of the changes are hopefully self-explanatory in light of the > documentation cited in generic-xlc.h. (Building on AIX has regressed in other > ways unrelated to atomics; I will write more about that in due course.) I wrote this entirely blindly, as evidenced here by the changes you needed. At the time somebody had promised to soon put up an aix animal, but that apparently didn't work out. Will you apply? Having the ability to test change seems to put you in a much better spot then me. > (2) "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, 5765-J08)" for ppc64le, > http://www-01.ibm.com/support/docview.wss?uid=swg27044056&aid=1. This > compiler has a Clang-derived C frontend. It defines __GNUC__ and offers > GCC-style __sync_* atomics. Phew. I don't see much reason to try to support this. Why would that be interesting? > Therefore, PostgreSQL selects generic-gcc.h. > test_atomic_ops() fails because __sync_lock_test_and_set() of one-byte types > segfaults at runtime. I have reported this to the vendor. Adding > "pgac_cv_gcc_sync_char_tas=no" to the "configure" invocation is a good > workaround. I could add a comment about that to src/test/regress/sql/lock.sql > for affected folks to see in regression.diffs. To do better, we could make > PGAC_HAVE_GCC__SYNC_CHAR_TAS perform a runtime test where possible. Yet > another option is to force use of generic-xlc.h on this compiler. It seems fair enough to simply add another test and include generic-xlc.h in that case. If it's indeed xlc, why not? > (3) "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, 5765-J08)" for ppc64le, > modifying atomics.h to force use of generic-xlc.h. While not a supported > PostgreSQL configuration, I felt this would make an interesting data point. > It worked fine after applying the patch developed for the AIX configuration. Cool. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] xlc atomics
On Wed, Jun 25, 2014 at 07:14:34PM +0200, Andres Freund wrote:
> * gcc, msvc work. acc, xlc, sunpro have blindly written support which
> should be relatively easy to fix up.
I tried this on three xlc configurations.
(1) "IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)". Getting it working
required the attached patch. None of my xlc configurations have an
header, and a web search turned up no evidence of one in connection with xlc
platforms. Did you learn of a configuration needing atomic.h under xlc? The
rest of the changes are hopefully self-explanatory in light of the
documentation cited in generic-xlc.h. (Building on AIX has regressed in other
ways unrelated to atomics; I will write more about that in due course.)
(2) "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, 5765-J08)" for ppc64le,
http://www-01.ibm.com/support/docview.wss?uid=swg27044056&aid=1. This
compiler has a Clang-derived C frontend. It defines __GNUC__ and offers
GCC-style __sync_* atomics. Therefore, PostgreSQL selects generic-gcc.h.
test_atomic_ops() fails because __sync_lock_test_and_set() of one-byte types
segfaults at runtime. I have reported this to the vendor. Adding
"pgac_cv_gcc_sync_char_tas=no" to the "configure" invocation is a good
workaround. I could add a comment about that to src/test/regress/sql/lock.sql
for affected folks to see in regression.diffs. To do better, we could make
PGAC_HAVE_GCC__SYNC_CHAR_TAS perform a runtime test where possible. Yet
another option is to force use of generic-xlc.h on this compiler.
(3) "IBM XL C/C++ for Linux, V13.1.2 (5725-C73, 5765-J08)" for ppc64le,
modifying atomics.h to force use of generic-xlc.h. While not a supported
PostgreSQL configuration, I felt this would make an interesting data point.
It worked fine after applying the patch developed for the AIX configuration.
Thanks,
nm
diff --git a/src/include/port/atomics/generic-xlc.h
b/src/include/port/atomics/generic-xlc.h
index 1c743f2..0ad9168 100644
--- a/src/include/port/atomics/generic-xlc.h
+++ b/src/include/port/atomics/generic-xlc.h
@@ -18,8 +18,6 @@
#if defined(HAVE_ATOMICS)
-#include
-
#define PG_HAVE_ATOMIC_U32_SUPPORT
typedef struct pg_atomic_uint32
{
@@ -48,9 +46,6 @@ static inline bool
pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
uint32
*expected, uint32 newval)
{
- boolret;
- uint64 current;
-
/*
* xlc's documentation tells us:
* "If __compare_and_swap is used as a locking primitive, insert a call
to
@@ -62,18 +57,15 @@ pg_atomic_compare_exchange_u32_impl(volatile
pg_atomic_uint32 *ptr,
* XXX: __compare_and_swap is defined to take signed parameters, but
that
* shouldn't matter since we don't perform any arithmetic operations.
*/
- current = (uint32)__compare_and_swap((volatile int*)ptr->value,
-
(int)*expected, (int)newval);
- ret = current == *expected;
- *expected = current;
- return ret;
+ return __compare_and_swap((volatile int*)&ptr->value,
+ (int *)expected,
(int)newval);
}
#define PG_HAVE_ATOMIC_FETCH_ADD_U32
static inline uint32
pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
{
- return __fetch_and_add(&ptr->value, add_);
+ return __fetch_and_add((volatile int *)&ptr->value, add_);
}
#ifdef PG_HAVE_ATOMIC_U64_SUPPORT
@@ -83,23 +75,17 @@ static inline bool
pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
uint64
*expected, uint64 newval)
{
- boolret;
- uint64 current;
-
__isync();
- current = (uint64)__compare_and_swaplp((volatile long*)ptr->value,
-
(long)*expected, (long)newval);
- ret = current == *expected;
- *expected = current;
- return ret;
+ return __compare_and_swaplp((volatile long*)&ptr->value,
+ (long
*)expected, (long)newval);;
}
#define PG_HAVE_ATOMIC_FETCH_ADD_U64
static inline uint64
pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
{
- return __fetch_and_addlp(&ptr->value, add_);
+ return __fetch_and_addlp((volatile long *)&ptr->value, add_);
}
#endif /* PG_HAVE_ATOMIC_U64_SUPPORT */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
