Re: atomic ops on i386

gcc defines the symbols __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 and __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
and I use code that looks like this:

#if defined(__GNUC__) && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 && __SIZEOF_LONG_LONG__ == 4
    (void) __sync_add_and_fetch( &longLongVar, 1 );
#elif defined(__GNUC__) && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 && __SIZEOF_LONG_LONG__ == 8
    (void) __sync_add_and_fetch( &longLongVar, 1 );
#else
    pthread_mutex_lock( &counterMutex ); // or other mutex lock not based on pthread
    longLongVar++;
    pthread_mutex_unlock( &counterMutex );
#endif

I think that the root cause of the problem is in src/ipc/AtomicWord.h where it is assumed
that if HAVE_ATOMIC_OPS is defined, atomic ops are defined for all types (and all sizes)
which is an incorrect assumption.

Changing the configure script to detect atomic ops for long long instead of int
is a workaround, but this prevents the use of atomic ops for 4-byte types on systems that support it.

Marcus

On 04/14/2014 11:45 AM, Alex Rousskov wrote:
> 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 - 17:54:49 MDT