On 02/01/2017 01:07 PM, Konstantin Knizhnik wrote:
> Attached please find my patch for XLC/AIX.
> The most critical fix is adding __sync to pg_atomic_fetch_add_u32_impl.
> The comment in this file says that:
>
> * __fetch_and_add() emits a leading "sync" and trailing "isync",
> thereby
> * providing sequential consistency. This is undocumented.
>
> But it is not true any more (I checked generated assembler code in
> debugger).
> This is why I have added __sync() to this function. Now pgbench working
> normally.
Seems like it was not so much undocumented, but an implementation detail
that was not guaranteed after all..
Does __fetch_and_add emit a trailing isync there either? Seems odd if
__compare_and_swap requires it, but __fetch_and_add does not. Unless we
can find conclusive documentation on that, I think we should assume that
an __isync() is required, too.
> Also there is mysterious disappearance of assembler section function
> with sync instruction from pg_atomic_compare_exchange_u32_impl.
> I have fixed it by using __sync() built-in function instead.
__sync() seems more appropriate there, anyway. We're using intrinsics
for all the other things in generic-xlc.h. But it sure is scary that the
"asm" sections just disappeared.
In arch-ppc.h, shouldn't we have #ifdef __IBMC__ guards for the __sync()
and __lwsync() intrinsics? Those are an xlc compiler-specific thing,
right? Or if they are expected to work on any ppc compiler, then we
should probably use them always, instead of the asm sections.
In summary, I came up with the attached. It's essentially your patch,
with tweaks for the above-mentioned things. I don't have a powerpc
system to test on, so there are probably some silly typos there.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers