From: "Wang, Zhihong" <zhihong.wang@intel.com>
To: "Wodkowski, PawelX" <pawelx.wodkowski@intel.com>,
"dev@dpdk.org" <dev@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
Date: Tue, 27 Jan 2015 05:12:03 +0000 [thread overview]
Message-ID: <F60F360A2500CD45ACDB1D700268892D0E7611C5@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <F6F2A6264E145F47A18AB6DF8E87425D12B8C8E2@IRSMSX102.ger.corp.intel.com>
> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Monday, January 26, 2015 10:43 PM
> To: Wang, Zhihong; dev@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)
next prev parent reply other threads:[~2015-01-27 5:12 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-19 1:53 [dpdk-dev] [PATCH 0/4] DPDK memcpy optimization zhihong.wang
2015-01-19 1:53 ` [dpdk-dev] [PATCH 1/4] app/test: Disabled VTA for memcpy test in app/test/Makefile zhihong.wang
2015-01-19 1:53 ` [dpdk-dev] [PATCH 2/4] app/test: Removed unnecessary test cases in test_memcpy.c zhihong.wang
2015-01-19 1:53 ` [dpdk-dev] [PATCH 3/4] app/test: Extended test coverage in test_memcpy_perf.c zhihong.wang
2015-01-19 1:53 ` [dpdk-dev] [PATCH 4/4] lib/librte_eal: Optimized memcpy in arch/x86/rte_memcpy.h for both SSE and AVX platforms zhihong.wang
2015-01-20 17:15 ` Stephen Hemminger
2015-01-20 19:16 ` Neil Horman
2015-01-21 3:18 ` Wang, Zhihong
2015-01-25 20:02 ` Jim Thompson
2015-01-26 14:43 ` Wodkowski, PawelX
2015-01-27 5:12 ` Wang, Zhihong [this message]
2015-01-19 13:02 ` [dpdk-dev] [PATCH 0/4] DPDK memcpy optimization Neil Horman
2015-01-20 3:01 ` Wang, Zhihong
2015-01-20 15:11 ` Neil Horman
2015-01-20 16:14 ` Bruce Richardson
2015-01-21 3:44 ` Wang, Zhihong
2015-01-21 11:40 ` Bruce Richardson
2015-01-21 12:02 ` Ananyev, Konstantin
2015-01-21 12:38 ` Neil Horman
2015-01-23 3:26 ` Wang, Zhihong
2015-01-21 12:36 ` Marc Sune
2015-01-21 13:02 ` Bruce Richardson
2015-01-21 13:21 ` Marc Sune
2015-01-21 13:26 ` Bruce Richardson
2015-01-21 19:49 ` Stephen Hemminger
2015-01-21 20:54 ` Neil Horman
2015-01-21 21:25 ` Jim Thompson
2015-01-22 0:53 ` Stephen Hemminger
2015-01-22 9:06 ` Luke Gorrie
2015-01-22 13:29 ` Jay Rolette
2015-01-22 18:27 ` Luke Gorrie
2015-01-22 19:36 ` Jay Rolette
2015-01-22 18:21 ` EDMISON, Kelvin (Kelvin)
2015-01-27 8:22 ` Wang, Zhihong
2015-01-28 21:48 ` EDMISON, Kelvin (Kelvin)
2015-01-29 1:53 ` Wang, Zhihong
2015-01-23 6:52 ` Wang, Zhihong
2015-01-26 18:29 ` Ananyev, Konstantin
2015-01-27 1:42 ` Wang, Zhihong
2015-01-27 11:30 ` Ananyev, Konstantin
2015-01-27 12:19 ` Ananyev, Konstantin
2015-01-28 2:06 ` Wang, Zhihong
2015-01-25 14:50 ` Luke Gorrie
2015-01-26 1:30 ` Wang, Zhihong
2015-01-26 8:03 ` Luke Gorrie
2015-01-27 7:19 ` Wang, Zhihong
2015-01-27 13:57 ` [dpdk-dev] [snabb-devel] " Luke Gorrie
2015-01-29 3:42 ` [dpdk-dev] " Fu, JingguoX
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=F60F360A2500CD45ACDB1D700268892D0E7611C5@SHSMSX101.ccr.corp.intel.com \
--to=zhihong.wang@intel.com \
--cc=dev@dpdk.org \
--cc=pawelx.wodkowski@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).