From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id A8306A496 for ; Sat, 13 Jan 2018 23:25:10 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 15A1120888; Sat, 13 Jan 2018 17:25:10 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Sat, 13 Jan 2018 17:25:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=7y55Hb5FihniDsvPKgqYklbh6m iQbj09ymfLp5Zm2R4=; b=G0P7aqUzM1sb6NyLqECkgqVhoJVc1PJh3GAapI+KMZ CSZ1IttvLciwL68l05h8S0bRApOt0jFrRVVdr3oqFTZoP15BLeqxWFAL3a61yShh NvDxy47eE64aE9QlLjaZ2HKAUtEf1buIa7B3KRl+YBpPtvNqyKZBY1l30sE74z95 o= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=7y55Hb 5FihniDsvPKgqYklbh6miQbj09ymfLp5Zm2R4=; b=P7HWlWX/salmI102kcyHnx DDRiVKEsOzPICqHZcVa68I6OUwGa2QrnkgTY/S4lEClYVOfbARQaw2k+fFVeubD9 +PjJN3sDnoNobY6O+KOQpr8LjGCxvtgy0tOQUyAfzMHppxm5FRbXLJmVXfD4/kKw FDhdpP2bhzFokPNwi83t8wUHIZZfKFY9sXZu+RyvLxhEV0FYQWFi20MVBBqmX8EA +00uXC6o68ivFkd/dsqJP9eslSwMtvxXPBiT+XF62LOMrCZs9PNGzb6rjToAYgiZ RZV2rmsmf3wmVs6EsEFd5CuQmf8G2uloP8AatYttM4n0tTK47RUZx4Gs5xHF/CcA == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 8727424741; Sat, 13 Jan 2018 17:25:09 -0500 (EST) From: Thomas Monjalon To: Aleksey Baulin Cc: dev@dpdk.org, "Wiles, Keith" Date: Sat, 13 Jan 2018 23:24:39 +0100 Message-ID: <216561584.1QX0fu7NSy@xps> In-Reply-To: References: <1511129764-23123-1-git-send-email-Aleksey.Baulin@gmail.com> <5180253.XvhpJrJZVt@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] eal/common: better likely() and unlikely() 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: Sat, 13 Jan 2018 22:25:10 -0000 Hi, I moved your top-post below and did some comments inline. More opinions are welcome. 13/01/2018 23:05, Aleksey Baulin: > On Fri, Jan 12, 2018 at 6:35 PM, Thomas Monjalon > wrote: > > 21/11/2017 08:05, Aleksey Baulin: > > > On Mon, Nov 20, 2017 at 4:36 PM, Wiles, Keith > > wrote: > > > > > On Nov 19, 2017, at 4:16 PM, Aleksey Baulin < > > aleksey.baulin@gmail.com> > > > > wrote: > > > > > -#define unlikely(x) __builtin_expect((x),0) > > > > > +#define unlikely(x) __builtin_expect(!!(x), 0) > > > > > > > > I have not looked at the generated code, but does this add some extra > > > > instruction now to do the !!(x) ? > > > > > > Sorry for late response. Jim had given the correct answer already. > > > You won't get an extra instruction with compiler optimization turned on. > > > > So this patch is adding an instruction in not optimized binary. > > I don't understand the benefit. > > Is it just to avoid to make pointer comparison explicit? > > likely(pointer != NULL) looks better than likely(pointer). > > This is an interesting question. Perhaps, even a philosophical one. :-) > > 'likely(pointer)' is a perfectly legal statement in C language, as well as > a concise one as > compared to a more explicit (and redundant/superfluous) 'likely(pointer != > NULL)'. If you > _require_ this kind of explicitness in cases like this in the code style, > then I have no > argument here. However, I don't see that anywhere. It is stated here: http://dpdk.org/doc/guides/contributing/coding_style.html#null-pointers > There're other cases of explicitness, with the most widespread being a > series of logical and > compare operations in one statement. For instance, 'if (a > b && a < c)'. > Explicitness would > require writing it like this: 'if ((a > b) && (a < c))'. I've seen cases on > this list where that was > frowned upon as it's totally unnecessary due to C operator precedence > rules, even though > those statements, I think, looked better to their authors (actually, they > do to me). Granted, > it didn't lead to compiler errors, which is the case with the current > implementation of 'likely()'. > > So, my answer to the question is yes, it's to avoid making pointer > comparison explicit. I would > add though, that it is to avoid making a perfectly legal C statement an > illegal one, as with the > way the current macro is constructed, compiler emits an error when DPDK is > built. I write in C > for many years with the most time spent in kernels, Linux and not, and I > find it unnatural to > always add a redundant '!= NULL' just to satisfy the current macro > implementation. I would > have to accept that though if it's a requirement clearly stated somewhere > like a code style. > > As for an extra instruction, I believe that everything in DPDK distribution > is compiled with > optimization. So the execution speed in not a concern here. Perhaps there > are cases where > it's compiled without optimization, like debugging, but then I believe it's > a non-issue. Yes you're right about optimization. But can we be 100% sure that it is always well optimized? > Hope my explanations shed some more light on this patch. :-) If we can be sure that there is no cost on optimized code, I am not against this patch. It may be especially useful when not complying to the DPDK coding rules, like in applications.