From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f177.google.com (mail-ie0-f177.google.com [209.85.223.177]) by dpdk.org (Postfix) with ESMTP id B6422C314 for ; Sat, 9 May 2015 01:25:10 +0200 (CEST) Received: by iecnq11 with SMTP id nq11so75030959iec.3 for ; Fri, 08 May 2015 16:25:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=WmWek6rfCayCRlS1mTjZG1YkXd2GXOO+T8DO9C/Y06g=; b=IVNOIhNp629290F3ghuEQYGv5KtMIcWmy4QiOKCQZGdXfTZKqQb06Lk3mfiGDwcffa 8RzR4rTgZypAKQXcsDeKmzj6AyODJk51X6v+C0OMBe1bRkuLGnjyumjPT9UA56Au/qvE x00PHnXuQvMP30aY8K2zqoFNatX1+5swU77IQCnirq8FJeT86YZN261hL+nke2ZhN0zX ZYyEyovbwlJPcGC7ewszvwXxLcRse0srKsaKMraOU+xusvIWW7gJIxPpXKqD4HJv0HYb aqast2Px5MMVy3IgtADguwocMcav/Jo/TsazFMZrQaqnMY8XJSFr5V45a+foEGjq1Sls 8nIw== X-Gm-Message-State: ALoCoQnlfP2CyZu6GHqrVcbkCtdeuCKxJrGB7VNy8yvpq/yaD23tDWEpqrt4F8BuhIGP4dVWkAka MIME-Version: 1.0 X-Received: by 10.42.20.14 with SMTP id e14mr570321icb.76.1431127510225; Fri, 08 May 2015 16:25:10 -0700 (PDT) Received: by 10.36.159.68 with HTTP; Fri, 8 May 2015 16:25:10 -0700 (PDT) In-Reply-To: References: <1431119946-32078-1-git-send-email-rkerur@gmail.com> <1431119989-32124-1-git-send-email-rkerur@gmail.com> Date: Fri, 8 May 2015 18:25:10 -0500 Message-ID: From: Matt Laswell To: Ravi Kerur Content-Type: text/plain; charset=UTF-8 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: Fri, 08 May 2015 23:25:11 -0000 On Fri, May 8, 2015 at 5:54 PM, Ravi Kerur wrote: > > > 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. > Ah, gotcha. I misunderstood and thought that this was meant to be a generic AVX/SSE enabled memcmp() replacement, and that the use of it in rte_hash was meant merely as a test case. If it's more limited than that, carry on, though you might want to make a note of it in the documentation. I suspect others will misinterpret the name as I did. -- Matt Laswell infinite io, inc. laswell@infiniteio.com