Fix typo 586/686 in atomics/arch-x86.h

Started by Jakub Wartak5 months ago23 messageshackers
Jump to latest
#1Jakub Wartak
jakub.wartak@enterprisedb.com

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.

Attachments:

v1-0001-Fix-typo-in-586-686-in-atomics-arch-x86.h.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Fix-typo-in-586-686-in-atomics-arch-x86.h.patchDownload+1-2
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Jakub Wartak (#1)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Daniel Gustafsson (#2)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#4Daniel Gustafsson
daniel@yesql.se
In reply to: John Naylor (#3)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#5Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Daniel Gustafsson (#4)
Re: Fix typo 586/686 in atomics/arch-x86.h

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.

#6John Naylor
john.naylor@enterprisedb.com
In reply to: Zsolt Parragi (#5)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: John Naylor (#6)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#8John Naylor
john.naylor@enterprisedb.com
In reply to: Tom Lane (#7)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#9John Naylor
john.naylor@enterprisedb.com
In reply to: Daniel Gustafsson (#4)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#10Zsolt Parragi
zsolt.parragi@percona.com
In reply to: John Naylor (#9)
Re: Fix typo 586/686 in atomics/arch-x86.h

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.

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zsolt Parragi (#10)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#12Andres Freund
andres@anarazel.de
In reply to: Daniel Gustafsson (#2)
Re: Fix typo 586/686 in atomics/arch-x86.h

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]/messages/by-id/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.gmail.com and
have a configure test for whether C11 atomics support doing 64bit reads/writes
without locks ([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). Unfortunately that requires a newer MSVC version and
specifying /experimental:c11atomics.

Greetings,

Andres Freund

[1]: /messages/by-id/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.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
64bit, to map to ATOMIC_LONG_LOCK_FREE / ATOMIC_LLONG_LOCK_FREE

#13Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: Andres Freund (#12)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#14Andres Freund
andres@anarazel.de
In reply to: Tomas Vondra (#13)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#15Nathan Bossart
nathandbossart@gmail.com
In reply to: Andres Freund (#12)
Re: Fix typo 586/686 in atomics/arch-x86.h

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

#16Jakub Wartak
jakub.wartak@enterprisedb.com
In reply to: Nathan Bossart (#15)
Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")

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.

#17Daniel Gustafsson
daniel@yesql.se
In reply to: Jakub Wartak (#16)
Re: Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")

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

#18Jakub Wartak
jakub.wartak@enterprisedb.com
In reply to: Daniel Gustafsson (#17)
Re: Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")

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.

#19Nathan Bossart
nathandbossart@gmail.com
In reply to: Jakub Wartak (#18)
Re: Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")

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:

/messages/by-id/flat/CO1PR07MB905262E8AC270FAAACED66008D682@CO1PR07MB9052.namprd07.prod.outlook.com

--
nathan

#20Yura Sokolov
y.sokolov@postgrespro.ru
In reply to: Jakub Wartak (#16)
Re: Drop 32-bit support (was "Re: Fix typo 586/686 in atomics/arch-x86.h")

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

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jakub Wartak (#16)
#22Andres Freund
andres@anarazel.de
In reply to: Jakub Wartak (#16)
#23Andres Freund
andres@anarazel.de
In reply to: Jakub Wartak (#18)