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