> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Monday, January 26, 2015 10:43 PM
> To: Wang, Zhihong; dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 4/4] lib/librte_eal: Optimized memcpy in
> arch/x86/rte_memcpy.h for both SSE and AVX platforms
>
> Hi,
>
> I must say: greate work.
>
> I have some small comments:
>
> > +/**
> > + * Macro for copying unaligned block from one location to another,
> > + * 47 bytes leftover maximum,
> > + * locations should not overlap.
> > + * Requirements:
> > + * - Store is aligned
> > + * - Load offset is <offset>, which must be immediate value within [1, 15]
> > + * - For <src>, make sure <offset> bit backwards & <16 - offset> bit
> forwards
> > are available for loading
> > + * - <dst>, <src>, <len> must be variables
> > + * - __m128i <xmm0> ~ <xmm8> must be pre-defined
> > + */
> > +#define MOVEUNALIGNED_LEFT47(dst, src, len, offset)
> > \
> > +{
> > \
> ...
> > +}
>
> Why not do { ... } while(0) or ({ ... }) ? This could have unpredictable side
> effects.
>
> Second:
> Why you completely substitute
> #define rte_memcpy(dst, src, n) \
> ({ (__builtin_constant_p(n)) ? \
> memcpy((dst), (src), (n)) : \
> rte_memcpy_func((dst), (src), (n)); })
>
> with inline rte_memcpy()? This construction can help compiler to deduce
> which version to use (static?) inline implementation or call external
> function.
>
> Did you try 'extern inline' type? It could help reducing compilation time.
Hi Pawel,
Good call on "MOVEUNALIGNED_LEFT47". Thanks!
I removed the conditional __builtin_constant_p(n) because it calls glibc memcpy
when the parameter is constant, while rte_memcpy has better performance there.
Current long compile time is caused by too many function calls, I'll fix that
in the next version.
Zhihong (John)
