From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) by dpdk.org (Postfix) with ESMTP id 6AB1C5683 for ; Wed, 2 Dec 2015 23:05:36 +0100 (CET) Received: by pabfh17 with SMTP id fh17so53850208pab.0 for ; Wed, 02 Dec 2015 14:05:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=63xz0wj5382RNucgn+McBAZtDLngDFeD7X8dw9bUHs8=; b=e940C8lkuKgdcroToaaC8Y/jml2NA6MxZ0MEp68cUUWeph5V/UOq5VlopMguE/2HJg c/tXNlxe0BRukm8cwJ8rnewYkaSJA3fQRMiql6WpSvuQCDEDu+MLW5RDj+C9k/EKy2tI HCv4dutMvTXHHTrEs7PCSvT/ejAnN3uiYn5aeNOfK0nRfNZ1QPSVxgMqeO5C4sz9UzjE aJhn2UeWh09nBJfCPc6tGglSD6yCxjnZc8ugNjgmEZc1XuXOa3lyEHKYBztfy1RuCNXH Ho/2izHCkQl28uaUqJIqqkwczUXH9e82mR1y8o6XmBVjighk7Bz0f730I/Z83GcsZlno eiyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=63xz0wj5382RNucgn+McBAZtDLngDFeD7X8dw9bUHs8=; b=mN/L1uag6QTbLvFh/07L8r/epSBhWcjyQ4Z/8mcovOrmNxMCRo5iP36aOTld4oKBTZ VMVpIByVf68nETrxbH4Myg6aBeV31/Il6cciIZkZve+6NQU0Okd6+BDZSEe3Tiu0srdV m8uqSYQIoZAbLALr85SnLg7xPg2mR210uCgYLX8+w7KGoVxuh0h1reKOESEFNp1pe0fT lgiowjL7E0KCK+bg1qefyi0k/Dh7+gJFbN/ADe1ZqcMt4/S3Qi8+GfKZQ+JzaLe9XTeh dxXadN2EqIHyF14LJa86BPFQuvuFWeMgTd55ODA0lT3MRtRNEjby92D1Tl+e6y1QMjhx RLqg== X-Gm-Message-State: ALoCoQmdYKHNXmdhmaL5yH7ad2BeBl7sQEgYxbUtrp/z2oZDSp420DJ1KpEaxTt7RfPJS1tdm8Wu X-Received: by 10.66.102.4 with SMTP id fk4mr8217233pab.85.1449093935682; Wed, 02 Dec 2015 14:05:35 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id fe6sm6286934pab.40.2015.12.02.14.05.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Dec 2015 14:05:35 -0800 (PST) Date: Wed, 2 Dec 2015 14:05:45 -0800 From: Stephen Hemminger To: "Dumitrescu, Cristian" Message-ID: <20151202140545.4e3c739b@xeon-e3> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D8912647925BB6@IRSMSX108.ger.corp.intel.com> References: <1448822809-8350-1-git-send-email-stephen@networkplumber.org> <1448822809-8350-3-git-send-email-stephen@networkplumber.org> <3EB4FA525960D640B5BDFFD6A3D8912647925BB6@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 2/3] rte_sched: introduce reciprocal divide 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: Wed, 02 Dec 2015 22:05:36 -0000 On Wed, 2 Dec 2015 16:45:01 +0000 "Dumitrescu, Cristian" wrote: > + * * Neither the name of Intel Corporation nor the names of its > > Why is Intel mentioned here, as according to this license header Intel is not the copyright holder? Copy/paste from other code. > > +#ifndef _RTE_RECIPROCAL_H_ > > +#define _RTE_RECIPROCAL_H_ > > + > > +struct rte_reciprocal { > > + uint32_t m; > > + uint8_t sh1, sh2; > > +}; > > The size of this structure is not a multiple of 32 bits. You seem to transfer this structure by value rather than by reference (the function rte_reciprocal_value() below returns an instance of this structure), I don't feel comfortable with the last 16 bits of the structure being left uninitialized, we should probably add some explicit pad field and initialize this structure explicitly to zero at init time? Shouldn't matter for inline at all. > > > + > > +static inline uint32_t rte_reciprocal_divide(uint32_t a, struct rte_reciprocal > > R) > > +{ > > + uint32_t t = (uint32_t)(((uint64_t)a * R.m) >> 32); > > + > > + return (t + ((a - t) >> R.sh1)) >> R.sh2; > > +} > > + > > +struct rte_reciprocal rte_reciprocal_value(uint32_t d); > > Why 32-bit arithmetic? We had a lot of bugs in librte_sched library due to 32-bit arithmetic that were particularly difficult to track. Can we have this function rte_reciprocal_divide() return a 64-bit integer and replace any 32-bit arithmetic/conversion with 64-bit operations? Doing reciprocal divide by multiply requires a 2x temporary. So if it used 64 bit math, it would require a 128 bit multiply. > > + > > +#endif /* _RTE_RECIPROCAL_H_ */ > > -- > > 2.1.4 > > As previously discussed, a simpler/faster alternative to floating point division is 64-bit multiplication followed by right shift, any particular reason why this approach was not considered? That is what this is. It is a 64 bit multiply (a * R.m) followed by a right shift. The only other stuff is related to round off and scaling. I chose to use known working algorithm rather than writing and having to do mathematical validation of any new code.