On 12.04.2017 16:01, Joe Savage wrote: > On 12/04/17 13:22, Dmitry Eremin-Solenikov wrote: >> On 12.04.2017 14:05, Joe Savage wrote: >>> On 12/04/17 11:32, Maxim Uvarov wrote: >>>> On 12.04.2017 13:15, Joe Savage wrote: >>>>>>>> The problem is that when we discussed this patch on ODP call people >>>>>>>> very >>>>>>>> worry about having 128bit instructions in ODP examples. At least Petri >>>>>>>> and Barry asked if it would be possible to rewrite that with 64 bit >>>>>>>> instructions? Some compilers might not support 128 bits and we need to >>>>>>>> test it more. >>>>>>> >>>>>>> On 32-bit platforms, it already does use 64-bit atomics. In general, >>>>>>> though, >>>>>>> the example hinges around having atomics that are twice the pointer >>>>>>> size. >>>>>>> We've actually discussed this on the list already in the thread "32-bit >>>>>>> support in examples". Even if lock-free implementations can't be used, >>>>>>> compilers can (and frequently do?) provide a lock-based compare exchange >>>>>>> operation. >>>>>> >>>>>> Any progress on this? >>>>> >>>>> This is getting mildly ridiculous now — it's nearing three months since I >>>>> initially submitted this simple example patch, and there's still no end in >>>>> sight! Maxim: any status updates? >>>>> >>>> >>>> Dmitry wanted to write some big review for that patch. But I do not see >>>> anything here. People commented on 128 bit instructions in examples and >>>> nobody set their review-by. I will rise question about your patch one >>>> more time on arch call. I can not include things where we did not get >>>> common agreement. I do not see anything bad with this patch but we need >>>> account all existence odp platforms. >> >> I'm sorry for the delay. Was quite under a pile of IPsec stuff... > > No worries. > >> I'm quite not happy about intermixture of arch-dependent and >> arch-specific stuff in headers. Would it be possible to split >> arch-specific implementation details to per-arch headers (one for >> AArch64, one for older ARM and one generic) and include proper header >> based on the $(ARCH_DIR)? It would be especially nice to have such >> header provide ATOMIC_STRONG_CAS_DBLPTR (or maybe just >> atomic_strong_cas_dblptr() ). > > Sure, I can split apart the architecture dependent stuff if you'd prefer it > that way. If I'm understanding you correctly, the suggestion here is to move > the architecture-specific CAS implementations into separate files — each > defining its own version of atomic_strong_cas_dblptr() — and for these files > to > then be conditionally included in odp_ipfragreass_helpers.h. Is that right?
Yes, that would be great! > > I'm not sure I quite follow your comments regarding $(ARCH_DIR) though. Are > there some ODP-defined macros that I can use instead of "__ARM_ARCH" and > friends or something? You can have arm, arm64, mips64, etc. directories inside examples/ipfrag. With smth. like odp_cas_dblptr.h header inside. Then make _helpers.h include odp_case_dblptr.h, adding -I$(ARCH_DIR) to CPPFLAGS in you example Makefile. -- With best wishes Dmitry
