[Gc] Re: Re[2]: libatomic_ops aarch64 support

Yvan Roux yvan.roux at linaro.org
Wed Feb 13 08:19:17 PST 2013


Hi Ivan,

(Added Marcus in CC)

As I said the issue with AO_double_load, is that we store two 64-bit
values and we don't load a a 128-bit one, because in
standard_ao_double_t.h, double_ptr_storage is defined as an unsigned
long long which 64-bit on AArch64. to fix this we have to defined it
as an __int128, but in that case the call to the builtin
__atomic_load_n is transformed by GCC to a call to __atomic_load_16
which is not implemented. The solution I see is to implement it has an
asm in aarch64.h with a ldxp (Load Exclusive Pair Registers
(extended)) instruction which loads two doublewords from memory and
records the physical address as an exclusive access :

AO_INLINE AO_double_t
AO_double_load(const volatile AO_double_t *addr)
{
  AO_double_t result;

  __asm__ __volatile__("//AO_double_load\n"
    "       ldxp %0, %1, [%2]"
    : "=&r" (result.AO_val1), "=&r" (result.AO_val2)
    : "r" (addr)
    /* : no clobber */);
  return result;
}
# define AO_HAVE_double_load

I tested it and seems to work, and if it is the good solution I can
implement the double_store/acquire/release the same way.

Thanks
Yvan


On 13 February 2013 00:15, Yvan Roux <yvan.roux at linaro.org> wrote:
> Hi Ivan,
>
>> What's the asm output for double_load? (in arm.h, you could see the
>> implementation for 32-bit ARM)
>
> the asm generated for AO_double_load is:
>
> ldr     x0, [x0]
> ret
>
> whereas it is an ldrexd in 32-bit ARM
>
> and the results of the test gives  new_w.val1 = 3316, new.val2 = 0
> because val1 is stored as 64-bit value :
>
> old_w.AO_val1 = 3316;  => str     x1, [x29,#64]
> old_w.AO_val2 = 2921;  => str     x2, [x29,#72]
> new_w = AO_double_load(&old_w); => ldr     x3, [x29,#64]
>
>> test_atomic_pthreads succeeds
>> test_malloc and test_stack abort
>>
>
>> It is ok for missing AO_char/short/int - I'll implement them using template
>> header.
>
> Ok
>
>> As I understand, you tested your first AArch64 implemented (that you
>> submitted to me), right?
>> If so, could you please identify some my commit that breaks
>> test_malloc/stack?
>
> Yes, the commit which brakes test_malloc/stack is:
> 64410803b698536fb0a3f714e3d1e582fb3d7630
>
> I'll look at the others point tomorrow ;)
>
> Cheers,
> Yvan
>
>
>
>
>>> Also I'd like to know asm generated for AO_nop_* primitives (I've changed
>>> __sync_synchronize to __atomic_thread_fence) and AO_double_* primitives.
>>
>> here is the generated code :
>>
>> AO_nop_read:
>>         dmb ishld // inner shareable load before load and load
>> before store
>>         ret
>>
>> AO_nop_write:
>>         dmb ish // inner shareable any before any
>>         ret
>>
>>
>> To my understanding, AO_nop_write should be defined as "dmb st". May be, I'm
>> wrong. Help of the community is really appreciated!
>>
>>
>>
>> AO_nop_full:
>>         dmb ish
>>         ret
>>
>>
>> Is this the same (or stronger) as for __sync_synchronize?
>>
>>
>>
>>
>>> Please rewrite CAS primitives (for AO_t and AO_double_t, relaxed and
>>> acquire/release/full variants) using __atomic_compare_exchange_n.
>>
>> ok fine, I will do it.
>>
>>> Q: Which compiler version do you use?
>>
>> I use for the cross compiler the
>> gcc-linaro-aarch64-linux-gnu-4.7-2013.01, I use my own build, but you
>> can find the binary release on the Linaro engineering armv8 page in
>> the download section (link below), and a 2012.12 native release (the
>> one shipped with the vexpress64-openembedded_sdk image) and with this
>> one the add-aarch64-support branch build failed during the compilation
>> of atomic_ops.c with the error:
>>
>> atomic_ops.c:96:1: error: unknown type name 'AO_TS_t'
>>
>>
>> Strange. This should be defined as AO_t in test_and_set_t_is_ao_t.h (please
>> check preprocessed code).
>>
>>
>>
>> http://www.linaro.org/engineering/armv8
>>
>>
>>> In case you have more free time (and wish to check some other targets), it
>>> would be good to check whether it is ok to use gcc/generic.h (starting
>>> from
>>> some GCC version) instead of asm-based primitives in gcc/*.h (by
>>> eye-inspection of asm code generated for the target comparing it to the
>>> instructions written in gcc/*.h files).
>>
>> I don't guarantee that I will check all the targets, but will give a look ;)
>>
>>
>> Thank you. I think it would be better to understand/fix the problems with
>> AArch64 first.
>>
>> Regards,
>> Ivan
>>
>>
>>
>>> Thank you. Sorry for a delay with the answer.
>>
>> No problem, many thanks
>>
>> Cheers,
>> Yvan
>>
>>> Regards,
>>> Ivan
>>>
>>> Fri, 8 Feb 2013, 16:53 +01:00 from Yvan Roux <yvan.roux at linaro.org>:
>>>
>>> Hi Ivan,
>>>
>>> Do you need more inputs regarding the aarch64 support, I didn't had
>>> time to work on this topic during the past weeks, but as I have a slot
>>> dedicated to that during the coming one, it would be great if you tell
>>> me what I can do.
>>>
>>> Cheers,
>>> Yvan
>>>
>>>


More information about the Gc mailing list