On Wed, Apr 12, 2017 at 8:24 AM, Dmitry Eremin-Solenikov <[email protected]> wrote: > 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.
This does not scale in practice. If another example comes along, does that also need to split whatever is determined to be "arch" code into "arch" directories within its own folder? I recommend that this contribution be accepted without any further change. > 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
