Re: pgsql: For all ppc compilers, implement pg_atomic_fetch_add_ with inlin

Noah Misch <noah@leadboat.com> writes:
> On Sat, Sep 14, 2019 at 10:10:47AM -0400, Tom Lane wrote:
>> prairiedog thinks there's something wrong here --- probably, some global
>> enabling #define is missing?

> I can reproduce its situation by commenting out the HAVE_GCC_ symbols in
> pg_config.h.  This commit did "#define PG_HAVE_ATOMIC_U32_SUPPORT" but defined
> only fetch_add, not compare_exchange.  That works with generic-gcc.h, but
> fallback.h can't replace just one.  (Rightly so -- fallback.h defines the
> pg_atomic_uint32 structure differently.)

Agreed on that diagnosis.

> I can see these ways to fix this:
> 1. Add pg_atomic_compare_exchange_u{32,64}_impl to arch-ppc.h
> 2. Arrange for arch-ppc.h to be a no-op when GCC atomics are unavailable
> 3. Just revert
> (1) seems best.  Since it may be days or weeks before I get to that, I'm
> inclined to revert after ~24h of total buildfarm exposure.

Yeah, seems like it should be possible to build atomic_compare_exchange
as an LWARX/STWCX loop, but it will take a little work.

I concur with waiting till your AIX flotilla has checked in before
reverting.  One thing we could find out that way is whether we actually
need the added configure test.  The returns so far say we don't:

 cavefish      | 2019-09-14 04:38:49 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 chub          | 2019-09-14 15:10:02 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 clam          | 2019-09-14 09:30:04 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 cuon          | 2019-09-14 06:57:23 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 demoiselle    | 2019-09-14 15:15:45 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 devario       | 2019-09-14 14:25:21 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 dhole         | 2019-09-14 05:46:33 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 elasmobranch  | 2019-09-14 03:18:50 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 gadwall       | 2019-09-14 12:39:48 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 prairiedog    | 2019-09-14 08:12:53 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 quokka        | 2019-09-14 08:46:25 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 shoveler      | 2019-09-14 14:44:30 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 takin         | 2019-09-14 08:49:27 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 tern          | 2019-09-14 09:51:04 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 urocryon      | 2019-09-14 06:45:31 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes
 vulpes        | 2019-09-14 09:35:41 | checking whether __builtin_constant_p(x) implies "i"(x) acceptance... yes

If the other AIX critters agree with tern, I'd be inclined to drop the
configure test and just make the code check for HAVE__BUILTIN_CONSTANT_P.

            regards, tom lane