Re: [PATCH] Add native windows on arm64 support - Mailing list pgsql-hackers

From Dave Cramer
Subject Re: [PATCH] Add native windows on arm64 support
Date
Msg-id CADK3HHJtcapYRTURFU5-p+3Dt-RYovbtyqCuoZXQbgAK0KEyWw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Add native windows on arm64 support  (Michael Paquier <michael@paquier.xyz>)
Responses Re: [PATCH] Add native windows on arm64 support
List pgsql-hackers

On Sat, 28 Sept 2024 at 20:03, Thomas Munro <thomas.munro@gmail.com> wrote:
On Tue, Feb 13, 2024 at 10:01 AM Dave Cramer <davecramer@postgres.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


Thomas,

Thanks, I will be able to try this when I get back from NY. So Thurs or so.

Dave 

pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Logical Replication of sequences
Next
From: Amit Kapila
Date:
Subject: Re: First draft of PG 17 release notes