Re: atomic ops on i386

On 04/13/2014 03:19 PM, Stuart Henderson wrote:
> On 2014-04-13, Alex Rousskov <rousskov_at_measurement-factory.com> wrote:
>> On 04/13/2014 06:36 AM, Stuart Henderson wrote:
>>
>>> I'm just trying to build 3.5-HEAD on OpenBSD/i386 (i.e. 32-bit mode) for
>>> the first time. It fails due to use of 64-bit atomic ops:
>>>
>>> MemStore.o(.text+0xc90): In function `MemStore::anchorEntry(StoreEntry&, int, Ipc::StoreMapAnchor const&)':
>>> : undefined reference to `__sync_fetch_and_add_8'
>>> MemStore.o(.text+0x3aa3): In function `MemStore::copyFromShm(StoreEntry&, int, Ipc::StoreMapAnchor const&)':
>>> : undefined reference to `__sync_fetch_and_add_8'
>>> MemStore.o(.text+0x3cce): In function `MemStore::copyFromShm(StoreEntry&, int, Ipc::StoreMapAnchor const&)':
>>> : undefined reference to `__sync_fetch_and_add_8'
>>> MemStore.o(.text+0x4040): In function `MemStore::copyFromShm(StoreEntry&, int, Ipc::StoreMapAnchor const&)':
>>> : undefined reference to `__sync_fetch_and_add_8'
>>> MemStore.o(.text+0x435f): In function `MemStore::copyFromShm(StoreEntry&, int, Ipc::StoreMapAnchor const&)':
>>> : undefined reference to `__sync_fetch_and_add_8'
>>> MemStore.o(.text+0x473d): more undefined references to `__sync_fetch_and_add_8' follow
>>> collect2: error: ld returned 1 exit status
>>
>> I am not an expert on this, but googling suggests building with
>> -march=i586 or a similar GCC option may solve your problem. More
>> possibly relevant details at
>
> That does fix the problem building, but I need this for package builds
> which are supposed to still work on 486, so I can't rely on users having
> 586 (cmpxchg8b).

>> http://www.squid-cache.org/mail-archive/squid-dev/201308/0103.html
>
> specifically "because swap_file_sz that they need to keep in sync
> across Squid kids is 64 bits" - so I think fixing the autoconf check is
> probably what's needed then.

Probably, assuming your users do not care about SMP-shared caching
(memory or disk).

>>> Should the autoconf test be changed to check for working 64-bit ops, or
>>> is something more involved wanted?
>>
>> Filing a bug report may be a good idea, especially if you cannot make
>> this work.
>
> I suppose the simplest fix would be something like this,
>
> --- configure.ac.orig Fri Apr 4 21:31:38 2014
> +++ configure.ac Sun Apr 13 15:12:37 2014
> @@ -416,7 +416,7 @@ dnl Check for atomic operations support in the compile
> dnl
> AC_MSG_CHECKING([for GNU atomic operations support])
> AC_RUN_IFELSE([AC_LANG_PROGRAM([[
> - int n = 0;
> + long long n = 0;
> ]],[[
> __sync_add_and_fetch(&n, 10); // n becomes 10
> __sync_fetch_and_add(&n, 20); // n becomes 30

Nitpick: s/long long/long long int/

but I think it would be safer to test both 32- and 64-bit sizes (using
two different variables instead of a single n). Also, ideally, we should
use int32_t and uint64_t types if possible, but that probably requires
#inclusion of other headers and may become difficult unless there are
already working ./configure test cases using those types that you can
copy from.

> Happy to open a bug report if that's preferred, I thought I'd ask here
> first to work out the best direction.

Again, I am not an expert on this, but I think you are on the right
track. If you can test your patch and post here, somebody will probably
commit it, especially if you can polish it based on the above suggestions.

A bug report just makes it easier to remember to do it (and to point
others to the fix).

Thank you,

Alex.
Received on Mon Apr 14 2014 - 14:45:21 MDT