Date: Mon, 15 Dec 2025 17:00:47 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Greg Burd <greg@burd.me>, Thomas Munro <thomas.munro@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Peter Eisentraut <peter@eisentraut.org>,
	Dave Cramer <davecramer@gmail.com>
Subject: Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by
 adding explicit DMB ?barriers
Message-ID: <aUCTH8oP4Wj4r58P@nathan>
References: <5e169c16-9e54-4325-8027-1f52a5bb5c1e@app.fastmail.com>
 <CA+hUKG+v8uqSibXXg5c1+qwJVY0nmpp_2YbTCpN8qrFwb+dvew@mail.gmail.com>
 <1c2a29b8-5b1e-44f7-a871-71ec5fefc120@app.fastmail.com>
 <f9bbd950-b70d-4881-9ae2-263e4d01e644@app.fastmail.com>
 <aTw8tb_eIpDBYHfW@nathan>
 <8e926f26-0a97-434f-a665-8bb6f9159b08@app.fastmail.com>
 <sxhkd5ucchq4vululj6xiwkrd6tsxh53gjiku2knjnqalglumc@hs4vp3b7e7by>
 <6646f9f4-3609-41e2-8d9f-dcd63e9f915e@app.fastmail.com>
 <aUB_6ZNFRtjM7FVS@nathan>
 <qu3uuja2u3uerwmbhvz6pscdhlhdm3xa3d4nc7lr7vvpowarzl@yn45ne6635ml>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="75SgnF0Jxi868KJh"
Content-Disposition: inline
In-Reply-To: <qu3uuja2u3uerwmbhvz6pscdhlhdm3xa3d4nc7lr7vvpowarzl@yn45ne6635ml>
Archived-At: <https://www.postgresql.org/message-id/aUCTH8oP4Wj4r58P%40nathan>
Precedence: bulk


--75SgnF0Jxi868KJh
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Dec 15, 2025 at 05:32:36PM -0500, Andres Freund wrote:
> On 2025-12-15 15:38:49 -0600, Nathan Bossart wrote:
>> -  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.

Done.

>> +#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".

Done.

>>  #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.

Done.

-- 
nathan

--75SgnF0Jxi868KJh
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v11-0001-Enable-the-Microsoft-Windows-ARM64-MSVC-platform.patch