Feedback-ID: i675e48f3:Fastmail
Date: Fri, 21 Nov 2025 14:52:03 -0500
From: Greg Burd <greg@burd.me>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Dave Cramer
 <davecramer@gmail.com>
Message-ID: <F83967FF-978A-4CC0-A6CB-752173E1F0F9@greg.burd.me>
In-Reply-To: 
 <CA+hUKGKm9bSkdfLZcNhcAJdRc=j4-CgXXSnyZ_p1697nvn7XYA@mail.gmail.com>
References: 
 <CA+hUKGKm9bSkdfLZcNhcAJdRc=j4-CgXXSnyZ_p1697nvn7XYA@mail.gmail.com>
Subject: Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on
 Win11 by adding explicit DMB =?utf-8?Q?=E2=80=8Bbarriers?=
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/F83967FF-978A-4CC0-A6CB-752173E1F0F9%40greg.burd.me>
Precedence: bulk


On Nov 20 2025, at 7:08 pm, Thomas Munro <thomas.munro=40gmail.com> wrote=
:

> On =46ri, Nov 21, 2025 at 9:45=E2=80=AFAM Greg Burd <greg=40burd.me> wr=
ote:
>> Dave and I have been working together to get ARM64 with MSVC functiona=
l.
>>  The attached patches accomplish that. Dave is the author of the first=

>> which addresses some build issues and fixes the spin=5Fdelay() semanti=
cs,
>> I did the second which fixes some atomics in this combination.
> =20
> A couple of immediate thoughts:

Hey Thomas, I'm always interested in your thoughts.  Thanks for taking a
look at this.

> https://learn.microsoft.com/en-us/cpp/intrinsics/interlockedexchangeadd=
-intrinsic-functions=3Fview=3Dmsvc-170
> =20
> Doesn't seem to match your conclusion.
> =20
> +  if cc.get=5Fid() =3D=3D 'msvc'
> +    cdata.set('USE=5FARMV8=5FCRC32C', false)
> +    cdata.set('USE=5FARMV8=5FCRC32C=5FWITH=5FRUNTIME=5FCHECK', 1)
> =20
> USE=5FARMV8=5FCRC32C=5FWITH=5FRUNTIME=5FCHECK (pg=5Fcrc32c=5Farmv8=5Fch=
oose.c) won't
> actually work on Windows, but I don't think we should waste time
> implementing it: vendor-supported versions of Windows 11 require
> ARMv8.1A to boot=5B1=5D=5B2=5D, and that has it, so I think we should p=
robably
> just define USE=5FARMV8=5FCRC32C.

I'll give that a go, I think your logic is sound.

> +static =5F=5Fforceinline void
> +spin=5Fdelay(void)
> +=7B
> +     /* Reference:
> https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics=23Bar=
rierRestrictions
> */
> +    =5F=5Fisb(=5FARM64=5FBARRIER=5FSY);
> +=7D
> =20
> I don't doubt that barriers are missing in a few places, but how can
> this be the right place=3F

Well, with the compiler flag that Andres mentioned I need to reconsider
this change.

> If you have an environment set up so it's easy to test, I would also
> be very interested to know if my patch set=5B3=5D that nukes all this
> stuff and includes <stdatomic.h> instead, which is green on
> Windows/x86 CI, will just work=E2=84=A2 there too.

I do, and I will as soon as I get this sorted.
 =20
> =5B1=5D https://en.wikipedia.org/wiki/Windows=5F11=5Fversion=5Fhistory
> =5B2=5D https://learn.microsoft.com/en-us/lifecycle/products/windows-11=
-home-and-pro
> =5B3=5D https://www.postgresql.org/message-id/flat/CA%2BhUKGK=46vu3zyvv=
3aaj5hHs9VtWcj=46AmisOwOc7aOZNc5A=463NA%40mail.gmail.com

best.

-greg