From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 4592B1B631 for ; Fri, 13 Oct 2017 09:31:24 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP; 13 Oct 2017 00:31:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,369,1503385200"; d="scan'208";a="909547664" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by FMSMGA003.fm.intel.com with ESMTP; 13 Oct 2017 00:31:22 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.49]) by IRSMSX108.ger.corp.intel.com ([169.254.11.167]) with mapi id 14.03.0319.002; Fri, 13 Oct 2017 08:31:21 +0100 From: "Ananyev, Konstantin" To: Thomas Monjalon , "Li, Xiaoyun" CC: "dev@dpdk.org" , "Richardson, Bruce" , "Lu, Wenzhuo" , "Zhang, Helin" Thread-Topic: [dpdk-dev] [PATCH v7 1/3] eal/x86: run-time dispatch over memcpy Thread-Index: AQHTPdZe6iJRcK0V+kC6fdxXRTu2daLbwMsAgAUxygCAAGjcAIAAESug Date: Fri, 13 Oct 2017 07:31:21 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772585FAA89A1@IRSMSX103.ger.corp.intel.com> References: <1507157911-8702-1-git-send-email-xiaoyun.li@intel.com> <35071535.8NSjKInSoG@xps> <1709550.5v5ZG7JxHL@xps> In-Reply-To: <1709550.5v5ZG7JxHL@xps> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjE4M2ZlZWMtOTRkMy00MjgyLWI0MzgtNmIwZmZkYjYyOTA1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjB4djdkeGdTa0RWUHpiUVh2VXppY21FMzk2N1ZqVUNzbEdUYlBNYlwvWDJVPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v7 1/3] eal/x86: run-time dispatch over memcpy X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Oct 2017 07:31:24 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Friday, October 13, 2017 8:22 AM > To: Li, Xiaoyun > Cc: dev@dpdk.org; Ananyev, Konstantin ; Ric= hardson, Bruce ; Lu, Wenzhuo > ; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH v7 1/3] eal/x86: run-time dispatch over me= mcpy >=20 > 13/10/2017 03:06, Li, Xiaoyun: > > Hi > > Sorry for the late reply. I took AL last 3 days. > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > 05/10/2017 14:33, Xiaoyun Li: > > > > +/** > > > > + * Macro for copying unaligned block from one location to another > > > > +with constant load offset, > > > > + * 47 bytes leftover maximum, > > > > + * locations should not overlap. > > > > + * Requirements: > > > > + * - Store is aligned > > > > + * - Load offset is , which must be immediate value within > > > > +[1, 15] > > > > + * - For , make sure bit backwards & <16 - offset> b= it > > > > +forwards are available for loading > > > > + * - , , must be variables > > > > + * - __m128i ~ must be pre-defined */ #define > > > > +MOVEUNALIGNED_LEFT47_IMM(dst, src, len, > > > > > > Naive question: > > > Is there a real benefit of using a macro compared to a static inline = function > > > optimized by a modern compiler? > > > > > The macro is in the existing DPDK codes. I didn't touch it. I just chan= ge the file name and the function name to rte_memcpy_internal. > > So I am not clear about if there is real benefit. > > In my opinion, I think it is the same as static inline function. > > > > Do I need to change them to inline function? >=20 > In this patch, it appears as a new macro. Ah no, it definitely been there before. All we did here - git mv rte_memcpy.h rte_memcpyu_interlan.h and then in rte_memcpy_internal.h renamed rte_memcpy() to rte_memcpy_intern= al(). > If you can, inline function is cleaner for the new one. I don't think it will be straightforward - one of the parameters is a const= ant value. My preference would be to keep original rte_memcpy() code intact as much as= we can here (except probably cosmetic changes - indentation, line length fixing etc.). After all that patch is for adding architecture function selection at runti= me only. If we like to improve our rte_memcpy() any furher - NP with that, but let i= t be a separate patch. Konstantin >=20 > > > Anyway, if you are doing a new version, please reduce lines length an= d fix > > > the indent from spaces to tabs. > > > > > They are original DPDK codes so I didn't touch them. > > But in next version, I will fix them. >=20 > Just to be sure: we are talking about fixing checkpatch warnings > only for the code added, changed or moved. >=20 > Thanks