From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f178.google.com (mail-ob0-f178.google.com [209.85.214.178]) by dpdk.org (Postfix) with ESMTP id 1D611594B for ; Mon, 11 May 2015 19:42:52 +0200 (CEST) Received: by obfe9 with SMTP id e9so105488118obf.1 for ; Mon, 11 May 2015 10:42:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=2oollFIk66ThcdILI9QMraywmK9E63GysyqE4R33ydU=; b=AlPhr35Av0kz//haFpYoS6x4o4Yz+nLj20KRiaXMULnzhDB9l6bEDY4S4PsGrYqCsc 8y0rcW71GSDKmBIfsqMEP/fyXuKjIO+mRjUUynU39woGbrB/Z5SIED8ZHyyoJt7IYEJ4 2W6W+wxcxDHNm4uJkMpaoYzEdvIFIj5i+T8hGTa3EmBPUJyhWS5j4sEyVCY/rKbIJQ1D uU32U4Ne4X6VenIh6lYEMSBWngFpnCQSrdd1vonXvFJ4ou7jMPkyA4vqwlc6EGEobbSX 5L/qERe16hCYYykSJ9kto3gxXnKSeFGTOwXJ3ELwJejUihky4WgkPsZJ9EPFF/Cj+Y19 ECKw== MIME-Version: 1.0 X-Received: by 10.60.56.97 with SMTP id z1mr9003204oep.59.1431366171453; Mon, 11 May 2015 10:42:51 -0700 (PDT) Received: by 10.202.179.195 with HTTP; Mon, 11 May 2015 10:42:51 -0700 (PDT) In-Reply-To: <2601191342CEEE43887BDE71AB9772582142E106@irsmsx105.ger.corp.intel.com> References: <1431119946-32078-1-git-send-email-rkerur@gmail.com> <1431119989-32124-1-git-send-email-rkerur@gmail.com> <2601191342CEEE43887BDE71AB9772582142E106@irsmsx105.ger.corp.intel.com> Date: Mon, 11 May 2015 10:42:51 -0700 Message-ID: From: Ravi Kerur To: "Ananyev, Konstantin" Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] Implement memcmp using AVX/SSE instructions. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 May 2015 17:42:52 -0000 Hi Konstantin, On Mon, May 11, 2015 at 2:51 AM, Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > Hi Ravi, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ravi Kerur > > Sent: Friday, May 08, 2015 11:55 PM > > To: Matt Laswell > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v2] Implement memcmp using AVX/SSE > instructions. > > > > On Fri, May 8, 2015 at 3:29 PM, Matt Laswell > wrote: > > > > > > > > > > > On Fri, May 8, 2015 at 4:19 PM, Ravi Kerur wrote: > > > > > >> This patch replaces memcmp in librte_hash with rte_memcmp which is > > >> implemented with AVX/SSE instructions. > > >> > > >> +static inline int > > >> +rte_memcmp(const void *_src_1, const void *_src_2, size_t n) > > >> +{ > > >> + const uint8_t *src_1 = (const uint8_t *)_src_1; > > >> + const uint8_t *src_2 = (const uint8_t *)_src_2; > > >> + int ret = 0; > > >> + > > >> + if (n & 0x80) > > >> + return rte_cmp128(src_1, src_2); > > >> + > > >> + if (n & 0x40) > > >> + return rte_cmp64(src_1, src_2); > > >> + > > >> + if (n & 0x20) { > > >> + ret = rte_cmp32(src_1, src_2); > > >> + n -= 0x20; > > >> + src_1 += 0x20; > > >> + src_2 += 0x20; > > >> + } > > >> > > >> > > > Pardon me for butting in, but this seems incorrect for the first two > cases > > > listed above, as the function as written will only compare the first > 128 or > > > 64 bytes of each source and return the result. The pattern expressed > in > > > the 32 byte case appears more correct, as it compares the first 32 > bytes > > > and then lets later pieces of the function handle the smaller remaining > > > bits of the sources. Also, if this function is to handle arbitrarily > large > > > source data, the 128 byte case needs to be in a loop. > > > > > > What am I missing? > > > > > > > Current max hash key length supported is 64 bytes, hence no comparison is > > done after 64 bytes. 128 bytes comparison is added to measure performance > > only and there is no use-case as of now. With the current use-cases its > not > > required but if there is a need to handle large arbitrary data upto 128 > > bytes it can be modified. > > So on x86 let say rte_memcmp(k1, k2, 65) might produce invalid results, > right? > While on PPC will work as expected (as it calls memcpu underneath)? > That looks really weird to me. > If you plan to use rte_memcmp only for hash comparisons, then probably > you should put it somewhere into librte_hash and name it accordingly: > rte_hash_key_cmp() or something. > And put a big comment around it, that it only works with particular > lengths. > If you want it to be a generic function inside EAL, then it probably need > to handle different lengths properly > on all supported architectures. > Konstantin > > Let me just explain it here and probably add it to document as well. rte_memcmp is not 1. a replacement to memcmp 2. restricted to hash key comparison rte_memcmp is 1. optimized comparison for 16 to 128 bytes, v1 patch series had this support. Changed some of the logic in v2 due to concerns raised for unavailable use-cases beyond 64 bytes comparison. With minor tuning over the weekend I am able to get better performance for anything between 16 to 128 bytes comparison. 2. will be specific to DPDK i.e. currently all memcmp usage in DPDK are for equality or inequality hence "less than" or "greater than" implementation in rte_memcmp doesn't make sense and will be removed in subsequent patches, it will return 0 or 1 for equal/unequal cases. rte_hash will be the first candidate to move to rte_memcmp and subsequently rte_lpm6 which uses 16 bytes comparison will be moved Later on RING_SIZE which uses large size for comparison will be moved. I am currently studying/understanding that logic and will make changes to rte_memcmp to support that. I don't want to make lot of changes in one shot and see that patch series die a slow death with no takers. Thanks, Ravi > > > > > > > -- > > > Matt Laswell > > > infinite io, inc. > > > laswell@infiniteio.com > > > > > > >