Re: [PATCH] Add native windows on arm64 support

On Sun, 29 Sept 2024 at 06:31, Dave Cramer <davecramer(at)postgres(dot)rocks>
wrote:

>
> On Sat, 28 Sept 2024 at 20:03, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
> wrote:
>
>> On Tue, Feb 13, 2024 at 10:01 AM Dave Cramer <davecramer(at)postgres(dot)rocks>
>> wrote:
>> > > postgres.exe!dsa_free(dsa_area * area, unsigned __int64 dp) Line 869 C
>> > postgres.exe!resize(dshash_table * hash_table, unsigned __int64
>> new_size_log2) Line 879 C
>> > postgres.exe!dshash_find_or_insert(dshash_table * hash_table, const
>> void * key, bool * found) Line 480 C
>> > postgres.exe!pgstat_get_entry_ref(PgStat_Kind kind, unsigned int
>> dboid, unsigned int objoid, bool create, bool * created_entry) Line 455 C
>>
>> Hmm, we can see that the spinlock stuff is too weak (assumes TSO), but
>> there aren't any spinlocks near that code so I think something else is
>> wrong. What does pg_read_barrier() compile to? I mention that
>> because it's used near there in a somewhat complicated way. But I'm a
>> bit confused because by reading through the code it looks like it
>> should be too strong, not too weak. I think it should fall back to
>> pg_memory_barrier_impl() for lack of pg_read_barrier_impl(), which
>> should produce MemoryBarrier() and I guess DMB SY (ARM instruction for
>> full system data memory barrier). So maybe some other pg_atomic_XXX
>> operations are falling back to code that doesn't work.
>>
>> Just for experimentation, you could see what happens if you redirect
>> all that stuff to C11's <stdatomic.h>. Here's a quick-and-dirty patch
>> for that, which works on my ARM Mac, but I have no idea if it'll work
>> on MSVC (our CI uses MSVC 2019; they only added <stdatomic.h> in MSVC
>> 2022, but that's the same edition that added ARM so you must have it).
>> It's not a very finely tuned patch (for example I think several calls
>> should use the _explicit() variants and be weaker, but they err on the
>> side of being too strong, so the code they generate is probably not as
>> fast as it could be; I lost interested when the general idea was
>> rejected for now, but it might help learn something about the MSVC+ARM
>> situation).
>>
>> Combined with the patch that redirects spinlocks to atomics[1], there
>> would be no 'hand rolled' code relating to memory order or atomics,
>> just standard modern C. There are obviously other architecture tests
>> here and there, and some of them will be wrong (some of them are wrong
>> already for MSVC on x86), and that might be fixed by project
>> standardisation[2].
>>
>> [1]
>> https://www.postgresql.org/message-id/CA%2BhUKGJ%2BoA%2B62iUZ-EQb5R2cAOW3Y942ZoOtzOD%3D1sQ05iNg6Q%40mail.gmail.com
>> [2]
>> https://www.postgresql.org/message-id/CA%2BhUKG%2BOu%2BC%2BpXioo_W1gKcwRCEN1WNrLBBPSMJyxjgc%2B1JEJA%40mail.gmail.com
>
>
>

Getting the following errors building it

FAILED: src/backend/nodes/nodefuncs.a.p/equalfuncs.c.obj
"ccache" "gcc" "-Isrc\backend\nodes\nodefuncs.a.p" "-Isrc\include\nodes"
"-I..\src\include\nodes" "-Isrc\include" "-I..\src\include"
"-I..\src\include\port\win32" "-IC:/Strawberry/c/include"
"-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall"
"-Winvalid-pch" "-O2" "-g" "-fno-strict-aliasing" "-fwrapv"
"-fexcess-precision=standard" "-Wmissing-prototypes" "-Wpointer-arith"
"-Werror=vla" "-Wendif-labels" "-Wmissing-format-attribute"
"-Wimplicit-fallthrough=3" "-Wcast-function-type"
"-Wshadow=compatible-local" "-Wformat-security"
"-Wdeclaration-after-statement" "-Wno-format-truncation"
"-Wno-stringop-truncation" "-pthread" "-DBUILDING_DLL" -MD -MQ
src/backend/nodes/nodefuncs.a.p/equalfuncs.c.obj -MF
"src\backend\nodes\nodefuncs.a.p\equalfuncs.c.obj.d" -o
src/backend/nodes/nodefuncs.a.p/equalfuncs.c.obj "-c"
../src/backend/nodes/equalfuncs.c
In file included from ..\src\include/port/atomics.h:180,
from ..\src\include/utils/dsa.h:17,
from ..\src\include/nodes/tidbitmap.h:26,
from ..\src\include/access/genam.h:19,
from ..\src\include/access/amapi.h:15,
from src\include\nodes/equalfuncs.funcs.c:18,
from ../src/backend/nodes/equalfuncs.c:88:
..\src\include/port/atomics/arch-x86.h:39: warning:
"pg_memory_barrier_impl" redefined
39 | #define pg_memory_barrier_impl() \
|
..\src\include/port/atomics.h:60: note: this is the location of the
previous definition
60 | #define pg_memory_barrier_impl()
atomic_thread_fence(memory_order_seq_cst)
|
..\src\include/port/atomics/arch-x86.h:44: warning: "pg_read_barrier_impl"
redefined
44 | #define pg_read_barrier_impl() pg_compiler_barrier_impl()
|
..\src\include/port/atomics.h:61: note: this is the location of the
previous definition
61 | #define pg_read_barrier_impl()
atomic_thread_fence(memory_order_acquire)
|
..\src\include/port/atomics/arch-x86.h:45: warning: "pg_write_barrier_impl"
redefined
45 | #define pg_write_barrier_impl() pg_compiler_barrier_impl()
|
..\src\include/port/atomics.h:62: note: this is the location of the
previous definition
62 | #define pg_write_barrier_impl()
atomic_thread_fence(memory_order_release)
|
..\src\include/port/atomics/arch-x86.h:66:3: error: conflicting type
qualifiers for 'pg_atomic_uint32'
66 | } pg_atomic_uint32;
| ^~~~~~~~~~~~~~~~
..\src\include/port/atomics.h:80:21: note: previous declaration of
'pg_atomic_uint32' with type 'pg_atomic_uint32' {aka '_Atomic unsigned int'}
80 | typedef atomic_uint pg_atomic_uint32;
| ^~~~~~~~~~~~~~~~
..\src\include/port/atomics/arch-x86.h: In function
'atomic_compare_exchange_strong':
..\src\include/port/atomics/arch-x86.h:181:43: error: request for member
'value' in something not a structure or union
181 | : "=a" (*expected), "=m"(ptr->value), "=q" (ret)
| ^~
..\src\include/port/atomics/arch-x86.h:182:55: error: request for member
'value' in something not a structure or union
182 | : "a" (*expected), "r" (newval), "m"(ptr->value)
| ^~
In file included from ..\src\include/port/atomics.h:57:
..\src\include/port/atomics/arch-x86.h: At top level:
..\src\include/port/atomics.h:94:38: error: expected declaration specifiers
or '...' before '(' token
94 | #define pg_atomic_fetch_add_u32_impl atomic_fetch_add
| ^~~~~~~~~~~~~~~~
..\src\include/port/atomics/arch-x86.h:189:1: note: in expansion of macro
'pg_atomic_fetch_add_u32_impl'
189 | pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32
add_)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
..\src\include/port/atomics.h:94:38: error: expected declaration specifiers
or '...' before '(' token
94 | #define pg_atomic_fetch_add_u32_impl atomic_fetch_add
| ^~~~~~~~~~~~~~~~
..\src\include/port/atomics/arch-x86.h:189:1: note: in expansion of macro
'pg_atomic_fetch_add_u32_impl'
189 | pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32
add_)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
..\src\include/port/atomics.h:94:38: error: expected declaration specifiers
or '...' before numeric constant
94 | #define pg_atomic_fetch_add_u32_impl atomic_fetch_add
| ^~~~~~~~~~~~~~~~
..\src\include/port/atomics/arch-x86.h:189:1: note: in expansion of macro
'pg_atomic_fetch_add_u32_impl'
189 | pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32
add_)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ..\src\include/port/atomics.h:201:
..\src\include/port/atomics/generic-gcc.h:30: warning:
"pg_compiler_barrier_impl" redefined
30 | #define pg_compiler_barrier_impl() __asm__ __volatile__("" :::
"memory")
|
..\src\include/port/atomics.h:65: note: this is the location of the
previous definition
65 | #define pg_compiler_barrier_impl()
atomic_signal_fence(memory_order_seq_cst)

Dave

>