On 12.04.2017 17:24, Brian Brooks wrote: > On Wed, Apr 12, 2017 at 8:24 AM, Dmitry Eremin-Solenikov > <dmitry.ereminsoleni...@linaro.org> 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.
>From my perspective, separate arm, aarch64 and generic headers would be sufficient, even if they are not put under separate directories. That was just a suggestion. >> 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 -- With best wishes Dmitry
