Обсуждение: Fix typo 586/686 in atomics/arch-x86.h
That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ If yes, then a patch is attached. Not that it harms something or somebody has such old hardware, but I've just spotted it while looking for something else. -J.
Вложения
> On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > > That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: > if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ > If yes, then a patch is attached. Not that it harms something or > somebody has such old hardware, but I've just spotted it while looking > for something else. That indeed looks like a clear typo, but if noone has complained since 2017 then maybe removing the checks is the right course of action? -- Daniel Gustafsson
On Fri, Nov 28, 2025 at 4:00 PM Daniel Gustafsson <daniel@yesql.se> wrote: > > > On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > > > > That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: > > if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ > > If yes, then a patch is attached. Not that it harms something or > > somebody has such old hardware, but I've just spotted it while looking > > for something else. > > That indeed looks like a clear typo, but if noone has complained since 2017 > then maybe removing the checks is the right course of action? I believe CI tests with -m32, so as long as we do that we should probably make that work the way we think it does. -- John Naylor Amazon Web Services
> On 19 Dec 2025, at 08:23, John Naylor <johncnaylorls@gmail.com> wrote: > > On Fri, Nov 28, 2025 at 4:00 PM Daniel Gustafsson <daniel@yesql.se> wrote: >> >>> On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: >>> >>> That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: >>> if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ >>> If yes, then a patch is attached. Not that it harms something or >>> somebody has such old hardware, but I've just spotted it while looking >>> for something else. >> >> That indeed looks like a clear typo, but if noone has complained since 2017 >> then maybe removing the checks is the right course of action? > > I believe CI tests with -m32, so as long as we do that we should > probably make that work the way we think it does. It does, but will this affect that? Does gcc change the CPU arch to 32bit era hardware when using -m32? I was under the impression that it built code that can run in 32-bit mode on the underlying hardware unless a specific target arch was defined - but this is outside my wheelhouse so I might well be uninformed. Regardless, applying this shouldn't affect anything unless compiling on Pentium Pro or pre-MMX Pentium instruction sets, so it seems quite harmless and as the intention was to support it the best course of action is probably to just apply this. -- Daniel Gustafsson
> It does, but will this affect that? Does gcc change the CPU arch to 32bit era > hardware when using -m32? I did some quick testing with this, normally only __i386__ gets defined for 32 bit builds (-march=native -m32 for example, but also the default -march=x86-64 -m32). __i586__ and __i686__ are only there if I pass the matching -march (i586/i686) flag to gcc.
On Fri, Dec 19, 2025 at 5:13 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > I did some quick testing with this, normally only __i386__ gets > defined for 32 bit builds (-march=native -m32 for example, but also > the default -march=x86-64 -m32). __i586__ and __i686__ are only there > if I pass the matching -march (i586/i686) flag to gcc. What platform is this? I don't see that: gcc 14: $ echo | gcc -m32 -dM -E - | grep -E '86[^0-9]' #define __i686 1 #define __i686__ 1 #define __i386 1 #define i386 1 #define __i386__ 1 clang 19: $ echo | clang -m32 -dM -E - | grep -E '86[^0-9]' #define __i386 1 #define __i386__ 1 #define i386 1 -- John Naylor Amazon Web Services
John Naylor <johncnaylorls@gmail.com> writes:
> On Fri, Dec 19, 2025 at 5:13 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
>> I did some quick testing with this, normally only __i386__ gets
>> defined for 32 bit builds (-march=native -m32 for example, but also
>> the default -march=x86-64 -m32). __i586__ and __i686__ are only there
>> if I pass the matching -march (i586/i686) flag to gcc.
> What platform is this? I don't see that:
I can replicate Zsolt's result --- note the point about -march:
$ echo | gcc -m32 -dM -E - | grep -E '86[^0-9]'
#define __i386 1
#define __i386__ 1
#define i386 1
$ echo | gcc -m32 -march=i586 -dM -E - | grep -E '86[^0-9]'
#define __i586 1
#define __tune_i586__ 1
#define __i386 1
#define __i586__ 1
#define __i386__ 1
#define i386 1
$ echo | gcc -m32 -march=i686 -dM -E - | grep -E '86[^0-9]'
#define __tune_i686__ 1
#define __i686 1
#define __i686__ 1
#define __i386 1
#define __i386__ 1
#define i386 1
This is with gcc 8.5.0 from RHEL8, and the same with gcc 14.3.1
from Fedora 41.
regards, tom lane
On Sat, Dec 20, 2025 at 9:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > John Naylor <johncnaylorls@gmail.com> writes: > > On Fri, Dec 19, 2025 at 5:13 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > >> I did some quick testing with this, normally only __i386__ gets > >> defined for 32 bit builds (-march=native -m32 for example, but also > >> the default -march=x86-64 -m32). __i586__ and __i686__ are only there > >> if I pass the matching -march (i586/i686) flag to gcc. > > > What platform is this? I don't see that: > > I can replicate Zsolt's result --- note the point about -march: > > $ echo | gcc -m32 -dM -E - | grep -E '86[^0-9]' > #define __i386 1 > #define __i386__ 1 > #define i386 1 > $ echo | gcc -m32 -march=i586 -dM -E - | grep -E '86[^0-9]' > #define __i586 1 > #define __tune_i586__ 1 > #define __i386 1 > #define __i586__ 1 > #define __i386__ 1 > #define i386 1 > $ echo | gcc -m32 -march=i686 -dM -E - | grep -E '86[^0-9]' > #define __tune_i686__ 1 > #define __i686 1 > #define __i686__ 1 > #define __i386 1 > #define __i386__ 1 > #define i386 1 > > This is with gcc 8.5.0 from RHEL8, and the same with gcc 14.3.1 > from Fedora 41. My results were from Fedora 41 gcc 14.3.1 as well. With '-march' I get the same as your 8.5.0 but in a slightly different order, but without it I still get some 'i686' symbols defined. -- John Naylor Amazon Web Services
On Fri, Dec 19, 2025 at 4:04 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > I believe CI tests with -m32, so as long as we do that we should
> > probably make that work the way we think it does.
>
> It does, but will this affect that? Does gcc change the CPU arch to 32bit era
> hardware when using -m32? I was under the impression that it built code that
> can run in 32-bit mode on the underlying hardware unless a specific target arch
> was defined - but this is outside my wheelhouse so I might well be uninformed.
It's outside mine as well, but when I saw this thread I thought of
Thomas's experiment to use <stdatomic.h> and the need to verify code
generation on various platforms, so I see this patch as preventing
some head-scratching with that effort. With this patch applied on a
non-debug -m32 build I do see changes with bloaty:
FILE SIZE VM SIZE
-------------- --------------
+0.3% +6.27Ki +0.3% +6.27Ki .eh_frame
-0.0% -16 -0.0% -16 .eh_frame_hdr
-0.0% -32 [ = ] 0 .symtab
-0.0% -47 [ = ] 0 .strtab
-0.0% -198 [ = ] 0 .debug_abbrev
-0.1% -512 [ = ] 0 .debug_rnglists
-34.3% -3.20Ki [ = ] 0 [Unmapped]
-0.1% -3.63Ki [ = ] 0 .debug_line
-0.1% -7.06Ki -0.1% -7.06Ki .text
-0.2% -8.44Ki [ = ] 0 .debug_loclists
-0.0% -8.70Ki [ = ] 0 .debug_info
-0.1% -25.5Ki -0.0% -824 TOTAL
--
John Naylor
Amazon Web Services
It seems this is dependent on the linux distribution. I assumed gcc uses the same march on all modern linux distributions, but that doesn't seem to be the case. OL 8/9/10, Gentoo, Arch seem to keep the x86-64 march even when you specify -m32: bash-5.2# gcc -m32 -Q --help=target | grep march -march= x86-64 Known valid arguments for -march= option: bash-5.2# gcc -Q --help=target | grep march -march= x86-64-v3 Known valid arguments for -march= option: But Ubuntu/Debian changes march to i386 when you do that: ❯ gcc -m32 -Q --help=target | grep march -march= i686 Known valid arguments for -march= option: ❯ gcc -Q --help=target | grep march -march= x86-64 Known valid arguments for -march= option: Gcc version doesn't seem to change this even if I install multiple gcc versions on the same setup.
Zsolt Parragi <zsolt.parragi@percona.com> writes:
> It seems this is dependent on the linux distribution. I assumed gcc
> uses the same march on all modern linux distributions, but that
> doesn't seem to be the case.
It may be changing with time, too. Same experiment on Red Hat distros:
gcc 8.5.0 on RHEL8:
$ gcc -Q --help=target | grep march
-march= x86-64
$ gcc -m32 -Q --help=target | grep march
-march= x86-64
gcc 11.5.0 on RHEL9:
$ gcc -Q --help=target | grep march
-march= x86-64-v2
Known valid arguments for -march= option:
$ gcc -m32 -Q --help=target | grep march
-march= x86-64
Known valid arguments for -march= option:
gcc 14.3.1 on Fedora 41:
$ gcc -Q --help=target | grep march
-march= x86-64
Known valid arguments for -march= option:
$ gcc -m32 -Q --help=target | grep march
-march= i686
Known valid arguments for -march= option:
regards, tom lane
Hi, On 2025-11-28 10:00:21 +0100, Daniel Gustafsson wrote: > > On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > > > > That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: > > if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ > > If yes, then a patch is attached. Not that it harms something or > > somebody has such old hardware, but I've just spotted it while looking > > for something else. > > That indeed looks like a clear typo, but if noone has complained since 2017 > then maybe removing the checks is the right course of action? Tomas also just found these typos, which made me find this thread. These typos are obviously mine. Ugh. I do think we should drop the 32bit support, rather than fixing the typos. While architecturally correct, the 586 indeed can do tear free 8 byte reads / writes, some quick experiments show that it's actually not entirely trivial to get the compiler to generate the right code, at least with gcc. A volatile 8 byte read / store with gcc only generates correct code when building with a newer -march= (it's using movq when correct, but it doesn't start using it with just -mmmx, which added the instruction). For 586 one needs to get the compiler to use fildq/fistpq, which I could only make happen when using the atomic builtins / C11 atomics. I also just can't get excited about expending any work on performance for 32bit builds. The proper and reliable way to do this would be to use C11 atomics [1] and have a configure test for whether C11 atomics support doing 64bit reads/writes without locks ([2]). Unfortunately that requires a newer MSVC version and specifying /experimental:c11atomics. Greetings, Andres Freund [1] https://postgr.es/m/CA%2BhUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA%40mail.gmail.com [2] Annoyingly that seems to require figuring out whether long or long long is 64bit, to map to ATOMIC_LONG_LOCK_FREE / ATOMIC_LLONG_LOCK_FREE
On 3/11/26 16:51, Andres Freund wrote: > Hi, > > On 2025-11-28 10:00:21 +0100, Daniel Gustafsson wrote: >>> On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: >>> >>> That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: >>> if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ >>> If yes, then a patch is attached. Not that it harms something or >>> somebody has such old hardware, but I've just spotted it while looking >>> for something else. >> >> That indeed looks like a clear typo, but if noone has complained since 2017 >> then maybe removing the checks is the right course of action? > > Tomas also just found these typos, which made me find this thread. > > These typos are obviously mine. Ugh. > > > I do think we should drop the 32bit support, rather than fixing the typos. > > > While architecturally correct, the 586 indeed can do tear free 8 byte reads / > writes, some quick experiments show that it's actually not entirely trivial to > get the compiler to generate the right code, at least with gcc. > > A volatile 8 byte read / store with gcc only generates correct code when > building with a newer -march= (it's using movq when correct, but it doesn't > start using it with just -mmmx, which added the instruction). For 586 one > needs to get the compiler to use fildq/fistpq, which I could only make happen > when using the atomic builtins / C11 atomics. > > I also just can't get excited about expending any work on performance for > 32bit builds. > True. Are you suggesting we "drop" the support even in backbranches? AFAIK those never actually supported this due to the typos, so it's not really a change in behavior. regards -- Tomas Vondra
Hi, On 2026-03-11 17:18:19 +0100, Tomas Vondra wrote: > On 3/11/26 16:51, Andres Freund wrote: > > On 2025-11-28 10:00:21 +0100, Daniel Gustafsson wrote: > >>> On 28 Nov 2025, at 09:44, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > >>> > >>> That's a typo in src/include/port/atomics/arch-x86.h, isn't it ?: > >>> if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ > >>> If yes, then a patch is attached. Not that it harms something or > >>> somebody has such old hardware, but I've just spotted it while looking > >>> for something else. > >> > >> That indeed looks like a clear typo, but if noone has complained since 2017 > >> then maybe removing the checks is the right course of action? > > > > Tomas also just found these typos, which made me find this thread. > > > > These typos are obviously mine. Ugh. > > > > > > I do think we should drop the 32bit support, rather than fixing the typos. > > > > > > While architecturally correct, the 586 indeed can do tear free 8 byte reads / > > writes, some quick experiments show that it's actually not entirely trivial to > > get the compiler to generate the right code, at least with gcc. > > > > A volatile 8 byte read / store with gcc only generates correct code when > > building with a newer -march= (it's using movq when correct, but it doesn't > > start using it with just -mmmx, which added the instruction). For 586 one > > needs to get the compiler to use fildq/fistpq, which I could only make happen > > when using the atomic builtins / C11 atomics. > > > > I also just can't get excited about expending any work on performance for > > 32bit builds. > > > > True. Are you suggesting we "drop" the support even in backbranches? > AFAIK those never actually supported this due to the typos, so it's not > really a change in behavior. I can see either just not touching the backbranches or applying the explicit removal there too. I don't really have a preference. Greetings, Andres Freund
On Wed, Mar 11, 2026 at 11:51:23AM -0400, Andres Freund wrote: > I do think we should drop the 32bit support, rather than fixing the typos. +1 > I also just can't get excited about expending any work on performance for > 32bit builds. +1. I'd go so far as to say that we should start removing all 32-bit-specific inline assembly, intrinsics, etc. from the tree. I doubt they're worth the complexity and maintenance costs. -- nathan
On Wed, Mar 11, 2026 at 6:50 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Wed, Mar 11, 2026 at 11:51:23AM -0400, Andres Freund wrote: > > I do think we should drop the 32bit support, rather than fixing the typos. > > +1 > > > I also just can't get excited about expending any work on performance for > > 32bit builds. > > +1. I'd go so far as to say that we should start removing all > 32-bit-specific inline assembly, intrinsics, etc. from the tree. I doubt > they're worth the complexity and maintenance costs. Hi all, I've marked this patch (to fix typos) as returned with feedback in CF. I'm +1 (mainly due to eliminating complexity and saving time for 'Linux - Debian Trixie - Meson' in Cirrus CI as it will be faster without testing the 32-bit stuff there; last run was like ~+7 mins for 'test_world_32' alone, and that's almost like 50% of the whole time there), but I think that the topic of removal 32-bit support deserves better transparency, so I'm sending this under new subject. I propose simply for now that that if there's consensus to drop the 32-bits support, then in the release notes of PG19 we could simply add some deprecation notice/warning like "PostgreSQL 19 is _probably_ the last release to provide support for 32-bits architectures. Please consider planning upgrade to 64-bit architecture." (and this costs us nothing, and gives any potential user additional year, and project would have even freedom to continue for couple releases still with 32-bits until somebody develops proper patch). The only trouble I see is that we should probalby excplictly continue to provide 32-bit client support (to allow embedded clients/IOT to continue). -J.
Re: Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")
От
Daniel Gustafsson
Дата:
> On 12 Mar 2026, at 08:57, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > ..I think that the topic of removal 32-bit support deserves better transparency, > so I'm sending this under new subject. My interpretation of the comments in the thread was to remove 32-bit support for 8 byte single-copy atomicity, not 32-bit support entirely. Maybe that was a misunderstanding but it seems like a better place to start given that it apparently doesn't work with no one complaining about it. -- Daniel Gustafsson
On Thu, Mar 12, 2026 at 2:33 PM Daniel Gustafsson <daniel@yesql.se> wrote: > > > On 12 Mar 2026, at 08:57, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > > > ..I think that the topic of removal 32-bit support deserves better transparency, > > so I'm sending this under new subject. > > > My interpretation of the comments in the thread was to remove 32-bit support > for 8 byte single-copy atomicity, not 32-bit support entirely. Maybe that was > a misunderstanding but it seems like a better place to start given that it > apparently doesn't work with no one complaining about it. > Right, you might be spot-on: I might have overreacted to this. But probably the main question is still valid (and now we have thread! :)). Should we maintain builds/testing for 32-bit PostgreSQL in 2026 and beyond? (Everyone else in IT world is getting rid of support for even 686). I remember researching if there any real 32-bit users out there and come up with nothing (maybe I'm wrong on this), but maybe that's the right moment to at least start deprecating 32-bits? -J.
On Thu, Mar 12, 2026 at 02:54:58PM +0100, Jakub Wartak wrote:
> Right, you might be spot-on: I might have overreacted to this. But probably
> the main question is still valid (and now we have thread! :)). Should we
> maintain builds/testing for 32-bit PostgreSQL in 2026 and beyond?
IMHO we should continue to maintain 32-bit support for now, but I don't
think we should bother micro-optimizing for those builds.
> I remember researching if there any real 32-bit users out there and come up
> with nothing (maybe I'm wrong on this), but maybe that's the right moment to at
> least start deprecating 32-bits?
I'm aware of at least one:
https://postgr.es/m/flat/CO1PR07MB905262E8AC270FAAACED66008D682%40CO1PR07MB9052.namprd07.prod.outlook.com
--
nathan
12.03.2026 10:57, Jakub Wartak пишет: > I propose simply for now that that if there's consensus to drop the 32-bits > support, then in the release notes of PG19 we could simply add some > deprecation notice/warning like "PostgreSQL 19 is _probably_ the last release > to provide support for 32-bits architectures. Please consider planning upgrade > to 64-bit architecture." (and this costs us nothing, and gives any potential > user additional year, and project would have even freedom to continue for > couple releases still with 32-bits until somebody develops proper patch). imho, it is possible to declare last version with 32bit support as LTS. i.e. support Pg18 or Pg19 for 10 years instead of 5 years. I'd vote for Pg18 to be last 32bit aware :))) -- regards Yura Sokolov aka funny-falcon
Jakub Wartak <jakub.wartak@enterprisedb.com> writes:
> I propose simply for now that that if there's consensus to drop the 32-bits
> support,
I do not believe there is any such consensus, and I for one am against it.
What was being discussed in the other thread was dropping some dedicated
code paths for 32-bit arches, which is in line with the general policy
that we've had for awhile of not optimizing for such builds anymore.
But there's a long way from that to "it won't work at all".
> The only trouble I see is that we should probalby excplictly continue to
> provide 32-bit client support (to allow embedded clients/IOT to continue).
Yes, that's one of the good reasons for not dropping it altogether.
*Maybe* there is an argument for not supporting 32-bit servers anymore,
but I don't really buy that. Also, how are you going to test a 32-bit
client build if the server has to be somewhere else? Building
infrastructure to support that would quickly eat up whatever win is
to be had.
regards, tom lane
Hi, On 2026-03-12 08:57:59 +0100, Jakub Wartak wrote: > I've marked this patch (to fix typos) as returned with feedback in CF. I'm +1 > (mainly due to eliminating complexity and saving time for > 'Linux - Debian Trixie - Meson' in Cirrus CI as it will be faster without > testing the 32-bit stuff there; last run was like ~+7 mins for 'test_world_32' > alone, and that's almost like 50% of the whole time there), but I think that > the topic of removal 32-bit support deserves better transparency, so I'm > sending this under new subject. The reason that takes substantial time is that it uses ubsan and alignment sanitizers. Even if we were to get rid of 32bit support - which I don't think we have concensus on - we wouldn't want to get rid of some of the coverage the test currently provides us. Greetings, Andres Freund
Hi, On 2026-03-12 14:54:58 +0100, Jakub Wartak wrote: > Right, you might be spot-on: I might have overreacted to this. But probably > the main question is still valid (and now we have thread! :)). Should we > maintain builds/testing for 32-bit PostgreSQL in 2026 and beyond? > > (Everyone else in IT world is getting rid of support for even 686). I'm confused the 686 is a good bit older than 64 bit support. i686 is from 1995, the first x86-64 chips are from 2003. And you can run 32bit postgres on a spanking new x86-64 CPU. So I don't understand how 686 desupport is related to 32bit support. Greetings, Andres Freund
> On 11 Mar 2026, at 16:51, Andres Freund <andres@anarazel.de> wrote: > I do think we should drop the 32bit support, rather than fixing the typos. Attached is a patch against HEAD for this. Reading your mail my interpretation was that this was equally broken for MSVC even though the macro is correct there, and we should remove both. Was that a correct reading? I'm not sure it's worth doing this to the backbranches, seems like we can keep this a v19+ fix. -- Daniel Gustafsson
Вложения
On Thu, May 07, 2026 at 10:59:26PM +0200, Daniel Gustafsson wrote: > Attached is a patch against HEAD for this. Reading your mail my interpretation > was that this was equally broken for MSVC even though the macro is correct > there, and we should remove both. Was that a correct reading? Yeah, atomics.h doesn't use the right macro for MSVC, so nothing in this file is compiled for MSVC anyway. However, I suspect the proper fix is to make sure this file is included for MSVC. Note that the other MSVC code in this file is similarly broken, but fortunately isn't used anywhere [0]. > I'm not sure it's worth doing this to the backbranches, seems like we can keep > this a v19+ fix. +1 [0] https://postgr.es/m/afouZUH_eUkIj4i4%40nathan -- nathan
On Thu, May 07, 2026 at 04:44:45PM -0500, Nathan Bossart wrote: > On Thu, May 07, 2026 at 10:59:26PM +0200, Daniel Gustafsson wrote: >> Attached is a patch against HEAD for this. Reading your mail my interpretation >> was that this was equally broken for MSVC even though the macro is correct >> there, and we should remove both. Was that a correct reading? > > Yeah, atomics.h doesn't use the right macro for MSVC, so nothing in this > file is compiled for MSVC anyway. However, I suspect the proper fix is to > make sure this file is included for MSVC. Note that the other MSVC code in > this file is similarly broken, but fortunately isn't used anywhere [0]. To be clear, I'm +1 on removing any 32-bit code in this file. But the 64-bit stuff should probably be fixed. -- nathan
> On 7 May 2026, at 23:46, Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Thu, May 07, 2026 at 04:44:45PM -0500, Nathan Bossart wrote: >> On Thu, May 07, 2026 at 10:59:26PM +0200, Daniel Gustafsson wrote: >>> Attached is a patch against HEAD for this. Reading your mail my interpretation >>> was that this was equally broken for MSVC even though the macro is correct >>> there, and we should remove both. Was that a correct reading? >> >> Yeah, atomics.h doesn't use the right macro for MSVC, so nothing in this >> file is compiled for MSVC anyway. However, I suspect the proper fix is to >> make sure this file is included for MSVC. Note that the other MSVC code in >> this file is similarly broken, but fortunately isn't used anywhere [0]. > > To be clear, I'm +1 on removing any 32-bit code in this file. But the > 64-bit stuff should probably be fixed. Agreed. Reading over the threads you linked it seems the way forward is to apply the patch posted here, and then fix arch detection so that atomics.h picks up the right code for x86-64. -- Daniel Gustafsson
On Fri, May 08, 2026 at 09:32:16AM +0200, Daniel Gustafsson wrote: > On 7 May 2026, at 23:46, Nathan Bossart <nathandbossart@gmail.com> wrote: >> To be clear, I'm +1 on removing any 32-bit code in this file. But the >> 64-bit stuff should probably be fixed. > > Agreed. Reading over the threads you linked it seems the way forward is to > apply the patch posted here, and then fix arch detection so that atomics.h > picks up the right code for x86-64. +1 -- nathan