That's good to know and certainly the code is cleaner that way. I think we should adopt the practice that any architecture-specific changes to linux-generic code (adds, updates, or removals) must either originate from or have a Reviewed-by a representative of that architecture (in this case, Bala). That way there's no confusion as to whether the known experts agree with the change, since these sort of things can have subtle implications that are not obvious to the layman.
On Tue, Dec 23, 2014 at 12:38 PM, Ola Liljedahl <[email protected]> wrote: > > 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 >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
