On Sun, 29 Sept 2024 at 06:31, Dave Cramer <davecramer@postgres.rocks> wrote:
> > On Sat, 28 Sept 2024 at 20:03, Thomas Munro <thomas.mu...@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 > > > 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 >
