I had some discussions with Bala and we think the GCC generates the appropriate code for OCTEON so there should be no need for any OCTEON-specific inline assembler in odp_atomic.h. I just didn't want to remove this piece of code when I redesigned the atomic support because I didn't understand why there is a SYNCW instruction *before* the load. Load-acquire type of loads may need sync or barrier instructions but they come after the load, not before.
I think we should trust GCC to generate better code for OCTEON than us mere humans, at least until proven wrong. So please remove this OCTEON-specific snippet. -- Ola On 23 December 2014 at 18:01, Mike Holmes <[email protected]> wrote: > > > On 23 December 2014 at 11:15, Taras Kondratiuk <[email protected]> > wrote: >> >> On 12/23/2014 05:28 PM, Mike Holmes wrote: >> > how will this work, Octeon does not care about __atomic_fetch_add(&v, >> > 1, __ATOMIC_RELAXED); and yet you will fail configure I assume >> > >> > static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *atom) >> > { >> > #if defined __OCTEON__ >> >>-------uint32_t ret; >> >>-------__asm__ __volatile__ ("syncws"); >> >>-------__asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret), "+m" (atom) : >> >>------->------->------- "r" (atom)); >> >>-------return ret; >> > #else >> >>-------return __atomic_fetch_add(&atom->v, 1, __ATOMIC_RELAXED); >> > #endif >> > } >> >> Linux-generic shouldn't have arch specific parts, because it is >> *generic*. Instead it can fall back to strong __sync functions if >> __atomic are not available. >> > > Linux kernel is generic but has core specifics - is this not the same ? > > What about these cases ? > > platform/linux-generic/odp_system_info.c:static int cpuinfo_octeon(FILE > *file, odp_system_info_t *sysinfo) > platform/linux-generic/odp_system_info.c: .cpu_arch_str = "octeon", > platform/linux-generic/odp_system_info.c: .cpuinfo_parser = > cpuinfo_octeon > platform/linux-generic/include/api/odp_align.h:#if defined __x86_64__ || > defined __i386__ > platform/linux-generic/include/api/odp_sync.h:#if defined __x86_64__ || > defined __i386__ > platform/linux-generic/include/odp_spin_internal.h:#if defined __x86_64__ || > defined __i386__ > platform/linux-generic/odp_system_info.c:#if defined __x86_64__ || defined > __i386__ || defined __OCTEON__ || \ > platform/linux-generic/odp_system_info.c:#if defined __x86_64__ || defined > __i386__ > platform/linux-generic/odp_system_info.c:static int cpuinfo_x86(FILE *file, > odp_system_info_t *sysinfo) > platform/linux-generic/odp_system_info.c: #if defined __x86_64__ || > defined __i386__ > platform/linux-generic/odp_system_info.c: .cpu_arch_str = "x86", > platform/linux-generic/odp_system_info.c: .cpuinfo_parser = > cpuinfo_x86 > platform/linux-generic/odp_system_info.c:#if defined __x86_64__ || defined > __i386__ || defined __OCTEON__ || \ > platform/linux-generic/odp_time.c:#if defined __x86_64__ || defined __i386__ > platform/linux-generic/include/api/odp_align.h:#elif defined __arm__ || > defined __aarch64__ > platform/linux-generic/include/api/odp_sync.h:#elif defined(__arm__) > platform/linux-generic/include/odp_spin_internal.h:#elif defined __arm__ > platform/linux-generic/odp_system_info.c:#elif defined __arm__ || defined > __aarch64__ > platform/linux-generic/odp_system_info.c:static int cpuinfo_arm(FILE *file > ODP_UNUSED, > platform/linux-generic/odp_system_info.c: #elif defined __arm__ || > defined __aarch64__ > platform/linux-generic/odp_system_info.c: .cpu_arch_str = "arm", > platform/linux-generic/odp_system_info.c: .cpuinfo_parser = > cpuinfo_arm > > >> >> >> Taras Kondratiuk > > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
