Re: LWLock optimization for multicore Power machines

On Sun, Mar 26, 2017 at 12:29 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> On Sat, Mar 25, 2017 at 11:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> writes:
>> > I moved PPC implementation of pg_atomic_fetch_mask_add_u32() into
>> > port/atomics/arch-ppc.h. I also had to declare pg_atomic_uint32 there
>> to
>> > satisfy usage of this type as argument
>> > of pg_atomic_fetch_mask_add_u32_impl().
>>
>> Hm, you did something wrong there, because now I get a bunch of failures:
>>
>> ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith
>> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
>> -Wformat-security -fno-strict-aliasing -fwrapv -g -O2
>> -I../../../../src/include -c -o brin.o brin.c
>> In file included from ../../../../src/include/port/atomics.h:123,
>> from ../../../../src/include/utils/dsa.h:17,
>> from ../../../../src/include/nodes/tidbitmap.h:26,
>> from ../../../../src/include/access/genam.h:19,
>> from ../../../../src/include/nodes/execnodes.h:17,
>> from ../../../../src/include/access/brin.h:14,
>> from brin.c:18:
>> ../../../../src/include/port/atomics/generic.h:154:3: error: #error "No
>> pg_atomic_test_and_set provided"
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_init_flag':
>> ../../../../src/include/port/atomics.h:178: warning: implicit
>> declaration of function 'pg_atomic_init_flag_impl'
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_test_set_flag':
>> ../../../../src/include/port/atomics.h:193: warning: implicit
>> declaration of function 'pg_atomic_test_set_flag_impl'
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_unlocked_test_flag':
>> ../../../../src/include/port/atomics.h:208: warning: implicit
>> declaration of function 'pg_atomic_unlocked_test_flag_impl'
>> ... and so on.
>>
>> I'm not entirely sure what the intended structure of these header files
>> is. Maybe Andres can comment.
>>
>
> It seems that on this platform definition of atomics should be provided by
> fallback.h. But it doesn't because I already defined PG_HAVE_ATOMIC_U32_SUPPORT
> in arch-ppc.h. I think in this case we shouldn't provide ppc-specific
> implementation of pg_atomic_fetch_mask_add_u32(). However, I don't know
> how to do this assuming arch-ppc.h is included before compiler-specific
> headers. Thus, in arch-ppc.h we don't know yet if we would find
> implementation of atomics for this platform. One possible solution is to
> provide assembly implementation for all atomics in arch-ppc.h.
>

BTW, implementation for all atomics in arch-ppc.h would be too invasive and
shouldn't be considered for v10.
However, I made following workaround: declare pg_atomic_uint32 and
pg_atomic_fetch_mask_add_u32_impl() only when we know that generic-gcc.h
would declare gcc-based atomics.
Could you, please, check it on Apple PPC?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company