public inbox for [email protected]  

help / color / mirror / Atom feed

From: Greg Burd <[email protected]>
To: Andres Freund <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Dave Cramer <[email protected]>
Subject: Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers
Date: Tue, 16 Dec 2025 07:40:13 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <qu3uuja2u3uerwmbhvz6pscdhlhdm3xa3d4nc7lr7vvpowarzl@yn45ne6635ml>
References: <[email protected]>
	<[email protected]>
	<CA+hUKG+v8uqSibXXg5c1+qwJVY0nmpp_2YbTCpN8qrFwb+dvew@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<aTw8tb_eIpDBYHfW@nathan>
	<[email protected]>
	<sxhkd5ucchq4vululj6xiwkrd6tsxh53gjiku2knjnqalglumc@hs4vp3b7e7by>
	<[email protected]>
	<aUB_6ZNFRtjM7FVS@nathan>
	<qu3uuja2u3uerwmbhvz6pscdhlhdm3xa3d4nc7lr7vvpowarzl@yn45ne6635ml>


On Mon, Dec 15, 2025, at 5:32 PM, Andres Freund wrote:
> Hi,
>
> On 2025-12-15 15:38:49 -0600, Nathan Bossart wrote:
>> +++ b/meson.build
>> @@ -2523,7 +2523,8 @@ int main(void)
>>  }
>>  '''
>>  
>> -  if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd without -march=armv8-a+crc',
>> +  if (host_cpu == 'aarch64' and cc.get_id() == 'msvc') or \
>> +        cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd without -march=armv8-a+crc',
>>        args: test_c_args)
>>      # Use ARM CRC Extension unconditionally
>>      cdata.set('USE_ARMV8_CRC32C', 1)
>
> I still think this should have a comment explaining that we can
> unconditionally rely on crc32 support due to window's baseline requirements.
>
>
>> diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
>> index 7f8f566bd40..e62141abf0a 100644
>> --- a/src/include/storage/s_lock.h
>> +++ b/src/include/storage/s_lock.h
>> @@ -602,13 +602,21 @@ typedef LONG slock_t;
>>  
>>  #define SPIN_DELAY() spin_delay()
>>  
>> -/* If using Visual C++ on Win64, inline assembly is unavailable.
>> - * Use a _mm_pause intrinsic instead of rep nop.
>> - */
>> -#if defined(_WIN64)
>> +#ifdef _M_ARM64
>> +static __forceinline void
>> +spin_delay(void)
>> +{
>> +	/* Research indicates ISB is better than __yield() on AArch64. */
>> +	__isb(_ARM64_BARRIER_SY);
>
> It'd be good to link the research in some form or another. Otherwise it's
> harder to evolve the code in the future, because we don't know if the research
> was "I liked the color better" or "one is catastrophically slower than the
> other".
>
>> +}
>> +#elif defined(_WIN64)
>>  static __forceinline void
>>  spin_delay(void)
>>  {
>> +	/*
>> +	 * If using Visual C++ on Win64, inline assembly is unavailable.
>> +	 * Use a _mm_pause intrinsic instead of rep nop.
>> +	 */
>>  	_mm_pause();
>>  }
>>  #else
>> @@ -621,12 +629,19 @@ spin_delay(void)
>>  #endif
>>  
>>  #include <intrin.h>
>> +#ifdef _M_ARM64
>> +#pragma intrinsic(_InterlockedExchange)
>> +
>> +/* _ReadWriteBarrier() is insufficient on non-TSO architectures. */
>> +#define S_UNLOCK(lock) _InterlockedExchange(lock, 0)
>> +#else
>>  #pragma intrinsic(_ReadWriteBarrier)
>>  
>>  #define S_UNLOCK(lock)	\
>>  	do { _ReadWriteBarrier(); (*(lock)) = 0; } while (0)
>>  
>>  #endif
>> +#endif
>
> The newline placement looks odd here. I'd add newlines around the #else.

Hey Andres, thanks for chiming in!  I agree with your suggestions and it looks like Nathan has already addressed them to your satisfaction.  I've applied v11 on my "unicorn" Win11 ARM64 MSVC system and it's testing now.

best.

-greg

> Greetings,
>
> Andres Freund

view thread (42+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox