Hi Ferruh,
> -----Original Message----- > From: Yigit, Ferruh > Sent: Thursday, March 21, 2019 1:37 AM > To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 6/8] net/ice: support Rx AVX2 vector > > On 3/18/2019 1:37 AM, Lu, Wenzhuo wrote: > > Hi Ferruh, > > > > > >> -----Original Message----- > >> From: Yigit, Ferruh > >> Sent: Saturday, March 16, 2019 1:54 AM > >> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > >> Subject: Re: [dpdk-dev] [PATCH v3 6/8] net/ice: support Rx AVX2 > >> vector > >> > >> On 3/15/2019 6:22 AM, Wenzhuo Lu wrote: > >>> Signed-off-by: Wenzhuo Lu <wenzhuo...@intel.com> > >> <...> > >> > >>> +#ifdef RTE_LIBRTE_ICE_16BYTE_RX_DESC > >>> + /* for AVX we need alignment otherwise loads are not > >> atomic */ > >>> + if (avx_aligned) { > >>> + /* load in descriptors, 2 at a time, in reverse order */ > >>> + raw_desc6_7 = _mm256_load_si256((void *)(rxdp + > >> 6)); > >>> + rte_compiler_barrier(); > >>> + raw_desc4_5 = _mm256_load_si256((void *)(rxdp + > >> 4)); > >>> + rte_compiler_barrier(); > >>> + raw_desc2_3 = _mm256_load_si256((void *)(rxdp + > >> 2)); > >>> + rte_compiler_barrier(); > >>> + raw_desc0_1 = _mm256_load_si256((void *)(rxdp + > >> 0)); > >>> + } else > >>> +#endif > >>> + do { > >>> + const __m128i raw_desc7 = > >>> + _mm_load_si128((void *)(rxdp + 7)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc6 = > >>> + _mm_load_si128((void *)(rxdp + 6)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc5 = > >>> + _mm_load_si128((void *)(rxdp + 5)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc4 = > >>> + _mm_load_si128((void *)(rxdp + 4)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc3 = > >>> + _mm_load_si128((void *)(rxdp + 3)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc2 = > >>> + _mm_load_si128((void *)(rxdp + 2)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc1 = > >>> + _mm_load_si128((void *)(rxdp + 1)); > >>> + rte_compiler_barrier(); > >>> + const __m128i raw_desc0 = > >>> + _mm_load_si128((void *)(rxdp + 0)); > >>> + > >>> + raw_desc6_7 = > >>> + _mm256_inserti128_si256 > >>> + > >> (_mm256_castsi128_si256(raw_desc6), > >>> + raw_desc7, 1); > >>> + raw_desc4_5 = > >>> + _mm256_inserti128_si256 > >>> + > >> (_mm256_castsi128_si256(raw_desc4), > >>> + raw_desc5, 1); > >>> + raw_desc2_3 = > >>> + _mm256_inserti128_si256 > >>> + > >> (_mm256_castsi128_si256(raw_desc2), > >>> + raw_desc3, 1); > >>> + raw_desc0_1 = > >>> + _mm256_inserti128_si256 > >>> + > >> (_mm256_castsi128_si256(raw_desc0), > >>> + raw_desc1, 1); > >>> + } while (0); > >> > >> Is this to provide the proper indention because of the above #ifdef > >> block? If so why not simple { } for the scope, is do{ }while(0) has benefit > against it? > > Yes, it's for the indention. To my opinion, "do while" looks friendly > > to the readers as we always use it in the macros. Only "{}" looks > > missing a function name or a "for ()" :) > > > > I found '{ }' more clear but no strong opinion, perhaps a comment to clarify > to intention can be good but that also looks like optional, so up to you. OK. I'll use '{}'. It also looks good to me.