On 25 March 2015 at 15:40, Sumith Dev Vojini <[email protected]> wrote:
> The compiler I have is not built with the correct arch which seems to be > the problem. If i pass -march=armv7-a compiler option during build the > assertion doesn't fail. > Thank you Ola! > I am glad the problem was solved. It is better that you get lock-less support for 64-bit atomics than the lock-based emulation. I will still have a look at ensuring that the linux-generic timer implementation works with e.g. PPC32. But as proven here, it is useful to get a warning that you don't get the best implementation. Don't know how to do that though. -- Ola > On Tue, Mar 24, 2015 at 4:19 AM, Ola Liljedahl <[email protected]> > wrote: > >> I downloaded the same GCC version from >> http://releases.linaro.org/14.04/components/toolchain/binaries/ >> $ >> /opt/gcc-linaro-arm-linux-gnueabihf-4.8-2014.04_linux/bin/arm-linux-gnueabihf-gcc >> --version >> arm-linux-gnueabihf-gcc (crosstool-NG linaro-1.13.1-4.8-2014.04 - Linaro >> GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) >> Same version but the output is slightly different. Did you crop your >> output? >> >> $ >> /opt/gcc-linaro-arm-linux-gnueabihf-4.8-2014.04_linux/bin/arm-linux-gnueabihf-gcc >> -dM -E - </dev/null | grep __GCC_ATOMIC_LLONG_LOCK_FREE >> #define __GCC_ATOMIC_LLONG_LOCK_FREE 2 >> As expected, GCC for ARMv7a supports atomic 64-bit operations without any >> helper locks. >> >> cc1 is where this symbol seems to be defined by the compiler. Do we have >> the same version? >> $ ls -l >> /opt/gcc-linaro-arm-linux-gnueabihf-4.8-2014.04_linux/libexec/gcc/arm-linux-gnueabihf/4.8.3/cc1 >> -rwxr-xr-x 1 minecraft minecraft 14093800 apr 16 2014 >> /opt/gcc-linaro-arm-linux-gnueabihf-4.8-2014.04_linux/libexec/gcc/arm-linux-gnueabihf/4.8.3/cc1 >> >> On 24 March 2015 at 05:26, Sumith Dev Vojini <[email protected]> >> wrote: >> >>> $ gcc --version >>> gcc (Linaro GCC 4.8-2014.04) 4.8.3 20140401 (prerelease) >>> >>> $ gcc -dM -E - < /dev/null | grep __GCC_ATOMIC_LLONG_LOCK_FREE >>> #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 >>> >>> Seems like __GCC ATOMIC_LLONG_LOCK_FREE is defined to be 1 and the lock >>> variable in odp_atomic_u64_s is included if it is less than 2. >>> >> There's something wrong with your setup. >> >> Without lock-less support for 64-bit atomics, performance will suffer. >> Get a compiler that has working support for 64-bit atomics on ARM. >> >> >>> >>> I also tried on PPCe500v2 cores which also failed the assertion. >>> >> Yes, e500 is 32-bit POWER architecture so does not support 64-bit atomics >> natively. There is no corresponding lwarx/stwcx paired unlike ARM. It >> should work fine on e5500/e6500 I assume. >> >> >>> On Mon, Mar 23, 2015 at 10:46 AM, Ola Liljedahl < >>> [email protected]> wrote: >>> >>>> On 23 March 2015 at 14:43, Sumith Dev Vojini <[email protected]> >>>> wrote: >>>> >>>>> >>>>> On Mon, Mar 23, 2015 at 5:31 AM, Ola Liljedahl < >>>>> [email protected]> wrote: >>>>> >>>>>> On 20 March 2015 at 19:15, Mike Holmes <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On 20 March 2015 at 14:10, Sumith Dev Vojini <[email protected] >>>>>>> > wrote: >>>>>>> >>>>>>>> Hello, >>>>>>>> >>>>>>>> While building ODP(v1.0.0) I encountered an assertion on the size of >>>>>>>> tick_buf_t in timer.c. The structure is >>>>>>>> >>>>>>>> typedef struct tick_buf_s { >>>>>>>> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ >>>>>>>> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not >>>>>>>> active */ >>>>>>>> #ifdef TB_NEEDS_PAD >>>>>>>> uint32_t pad;/* Need to be able to access padding for >>>>>>>> successful CAS */ >>>>>>>> #endif >>>>>>>> } tick_buf_t >>>>>>>> #ifdef ODP_ATOMIC_U128 >>>>>>>> ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned >>>>>>>> addresses */ >>>>>>>> ; >>>>>>>> _ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == >>>>>>>> 16"); >>>>>>>> >>>>>>>> >>>>>>>> odp_atomic_u64_t is of type struct odp_atomic_u64_s >>>>>>>> >>>>>>>> struct odp_atomic_u64_s { >>>>>>>> uint64_t v; /**< Actual storage for the atomic variable */ >>>>>>>> #if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >>>>>>>> /* Some architectures do not support lock-free operations >>>>>>>> on 64-bit >>>>>>>> * data types. We use a spin lock to ensure atomicity. */ >>>>>>>> char lock; /**< Spin lock (if needed) used to ensure atomic >>>>>>>> access */ >>>>>>>> #endif >>>>>>>> } ODP_ALIGNED(sizeof(uint64_t)); /* Enforce alignement! */; >>>>>>>> >>>>>>>> So the systems which cannot do atomic 64 bit operations will have >>>>>>>> lock variable defined which increases the size of the that structure >>>>>>>> odp_atomic_u64_s to 16 bytes due to the alignment requirements and >>>>>>>> the assertion fails since tick_buf_s has two more variables >>>>>>>> increasing its size to 24 bytes. >>>>>>>> >>>>>>> Since the size and especially alignment is important to the timer >>>>>> implementation, the best fix is to ensure that odp_atomic_64_t is >>>>>> actually >>>>>> only 64 bits. This could be achieved by moving the lock variable out from >>>>>> the atomic variable into a separate array (equivalent to how timer.c is >>>>>> doing it when the target does not support 128-bit atomics). Each 64-bit >>>>>> atomic does not need its own lock variable, that's mostly a waste of >>>>>> space. >>>>>> Create a N-to-1 mapping to a smaller set of spin locks and use them when >>>>>> accessing the 64-bit atomic variable. >>>>>> >>>>>> Alternatively, let timer.c handle all synchronization internally and >>>>>> use uint64_t instead of odp_atomic_u64_t. >>>>>> >>>>>> BTW: which target is this that does not support 64-bit atomics >>>>>> natively? >>>>>> >>>>> >>>>> >>>>>> I am working on Freescale's LS1021A with cortex A7. >>>>>> >>>>> Cortex-A7 is ARMv7a like Cortex-A15. >>>> The current atomics and timer implementation works with ARMv7a, with >>>> native 64-bit atomics support (using load/store exclusive paired). Why >>>> doesn't this for work for you? I was expecting you to use something based >>>> on PPC32 which doesn't support 64-bit atomics. >>>> >>>> -- Ola >>>> >>>> >>>> >>>> >>>>>> >>>>>> >>>>>>>> >>>>>>> Thanks Sumith >>>>>>> >>>>>>> Could you make a bug report ? >>>>>>> >>>>>>> https://bugs.linaro.org/enter_bug.cgi?product=OpenDataPlane >>>>>>> >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Regards >>>>>>>> Sumith Dev >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> lng-odp mailing list >>>>>>>> [email protected] >>>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Mike Holmes >>>>>>> Technical Manager - Linaro Networking Group >>>>>>> Linaro.org <http://www.linaro.org/> *│ *Open source software for >>>>>>> ARM SoCs >>>>>>> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> lng-odp mailing list >>>>>>> [email protected] >>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Regards >>>>> SD >>>>> >>>> >>>> >>> >>> >>> -- >>> Regards >>> SD >>> >> >> > > > -- > Regards > SD >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
