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(dot)wartak(at)enterprisedb(dot)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