Feedback-ID: i675e48f3:Fastmail
Date: Sun, 23 Nov 2025 16:17:25 -0500
From: Greg Burd <greg@burd.me>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Message-ID: <CC76554F-41AC-45AA-AF10-370FEC416498@greg.burd.me>
In-Reply-To: 
 <CA+hUKGJSx124Y1gFJi2y_eRFpO=Ubcdi71-iEn6nmBQjbZFZ+g@mail.gmail.com>
References: 
 <CA+hUKGJSx124Y1gFJi2y_eRFpO=Ubcdi71-iEn6nmBQjbZFZ+g@mail.gmail.com>
Subject: Re: Trying out <stdatomic.h>
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/CC76554F-41AC-45AA-AF10-370FEC416498%40greg.burd.me>
Precedence: bulk


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

> On Mon, Nov 24, 2025 at 4:23=E2=80=AFAM Greg Burd <greg=40burd.me> wrot=
e:
>> As mentioned on a separate thread about fixing ARM64 support when
>> building with MSVC on Win11 =5B1=5D I tried out this patch.  The reply=
 on
>> that thread had an issue with =5Fmm=5Fpause() in spin=5Fdelay(), it tu=
rns
>> out we need to use =5F=5Fyield() =5B2=5D.  I went ahead and fixed that=
, so
>> ignore that patch on the other thread =5B1=5D.  The new patch attached=

>> that layers on top of your work and supports that platform, there was
>> one minor change that was required:
>> =20
>> =20
>> =23ifdef =5FMSC=5FVER
>> =20
>>         /*
>>          * If using Visual C++ on Win64, inline assembly is
>> unavailable.  Use a
>>          * =5Fmm=5Fpause intrinsic instead of rep nop. =46or ARM64, us=
e the =5F=5Fyield()
>>          * intrinsic which emits the YIELD instruction as a hint to
>> the processor.
>>          */
>> =23if defined(=5FM=5FARM64)
>>         =5F=5Fyield();
>> =23elif defined(=5FWIN64)
>>         =5Fmm=5Fpause();
>> =23else
>>         /* See comment for gcc code. Same code, MASM syntax */
>>         =5F=5Fasm           rep nop;
>> =23endif
>> =23endif                                                  /* =5FMSC=5F=
VER */
> =20
> That makes more intuitive sense... but I didn't know that people *do*
> sometimes prefer instruction synchronisation barriers for spinlock
> delays:
> =20
> https://stackoverflow.com/questions/70810121/why-does-hintspin-loop-use=
-isb-on-aarch64
> =20
> When reading your patch I was pretty confused by that, because it said
> it was fixing a barrier problem and apparently doing so in an
> unprincipled place.  I guess we really need to research the best delay
> mechanism for our needs on this architecture, independently of the
> compiler being used, and then write matching GCC and Visual Studio
> versions of that=3F  I think there were some threads about spinlock
> performance on Linux + Graviton with graphs and theories...

Interesting, I think I was rushing to get past that compile issue rather
than optimizing.  This sounds like yet another place where we should
choose based on arch and it seems hint::spin=5Floop() does.

>> tests are passing, best.
> =20
> Great news=21  Thanks.  It sounds like if I could supply the missing
> credible evidence of codegen quality on... all the computers, then I
> think we'd be down to just: when can we pull the trigger and require
> Visual Studio 2022 and do we trust /experimental:c11atomics=3F

I'm in favor of the stdatomic approach.  I can't speak to codegen
quality on *all the platforms* or how *experimental* c11 atomics are
when using MSVC.

> =46TR I had earlier shared some version of this patch with Dave when he=

> was trying to get his Windows/ARM system going, but I think my earlier
> version was probably too broken.  Sorry Dave.  At that stage I was
> also trying to do it as an option but keeping the existing stuff
> around.  Since then we adopted C11, so this is the all-in version.  I
> also hadn't understood a key part of the C11 memory model that your
> RISC-V animal taught me and that c5d34f4a fixed, and you can see in
> this patch set too, and I'm not sure if Visual Studio is like GCC or
> Clang in that respect.

Thanks for that work on RISC-V, I appreciate that=21  Much more digging t=
o
be done to answer those questions for sure.

> It crossed my mind that this might even be
> related to the problem you've noticed with barriers being missing, but
> I haven't looked into that.  BTW I believe we could actually change
> our code NOT to rely on that, ie to follow the C11 memory model better
> and declare eg PgAioHandle::status as atomic=5Fuint8 or whatever (other=

> non-atomic access would be considered dependent and do the right thing
> IIUC), but I'm not sure if it's necessary and that research project
> can wait.

Interesting.  Yeah, let's do one thing and then move to the next, but I
do like the idea.

best.

-greg