From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f68.google.com (mail-pl0-f68.google.com [209.85.160.68]) by dpdk.org (Postfix) with ESMTP id 5F9392D13 for ; Sun, 14 Jan 2018 18:17:18 +0100 (CET) Received: by mail-pl0-f68.google.com with SMTP id b96so2088065pli.2 for ; Sun, 14 Jan 2018 09:17:18 -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-transfer-encoding; bh=7DoiLpcafd//tq/Dztfg9uJtO6t1KVYPqUC6YdI1bg4=; b=yaE/iJT9rEd67SYYrIiufzu6dcjzjv12Lu74S7YyLp8iQ2wJde0UMN8PmD/CSV5OfE 5yTfVJ3NZuSUW/vW+HVn83FZaiNQhlWYkl0C11owX3049fx5DZXwlyrYiTz0cM4lWqtw EbzKXydcofNLBDDT+vq0/kmJQFweIXyOjAcJrbCu92gYjf6Y9Zwe8X48Ur2eDYNKhC60 72iTKZ8cR4ePASzyDUoMu3K3Z7fwyc11K8Bw8wVbrAmA2zxkWdqF5qS5eDMwJsxO2GT1 Km4UrGxNVjdqet9GSaRIO6CyTecOuPNSED5+C/QcX5sDMqxIiFkNtP2hPOBvTcEfMsOk 0wuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=7DoiLpcafd//tq/Dztfg9uJtO6t1KVYPqUC6YdI1bg4=; b=j0HvLyxJdNTtVqXN60xHxVnfvpkFmlY8LYRCojoDvWOqMLzlmRKxo+wVI8vzMMHQrU BKPmcKiXpKOUwRxyuNk6ZvyIUm50YoxUMqYMHXWYTq6mCjXgTmPNn9PwU3LlqgMo0q5a YDuq93x24g8O4iXjxxGGIUiKn7e6OoUzlE5hLrM+wBx6fFr06iM1pWBXOEEkOt2z0NW6 UE6STNOigVX0IEfBhaLI1JUgKPaEF07D+Awy8IWcazRYHgTcW0cOrYnhNsUiTLzs+Tos WmdmG4bDTZ/XCYYapC4mpP+zFI5EdMQC6X9y/CNoKxrCzu0B3gux8JxdtXPvPw0G1P4Q Bv+A== X-Gm-Message-State: AKGB3mKIk1Y3hgQuoE6qkM5eFi14nkJxJPkRcYKoPalTA0SahRJq2Zlc +19u0z8j9d2N2A5Lgk5IUjKsqA== X-Google-Smtp-Source: ACJfBouN15qrEJjoxyLYZc69Tr499GITDLHwg4ePMs+jw4Mo1LJfz44luNgcZ5vmOmEkjJefbq4Zdw== X-Received: by 10.159.246.135 with SMTP id c7mr25545844pls.138.1515950237502; Sun, 14 Jan 2018 09:17:17 -0800 (PST) Received: from xeon-e3 (204-195-18-133.wavecable.com. [204.195.18.133]) by smtp.gmail.com with ESMTPSA id d186sm39456885pfa.0.2018.01.14.09.17.17 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 14 Jan 2018 09:17:17 -0800 (PST) Date: Sun, 14 Jan 2018 09:17:09 -0800 From: Stephen Hemminger To: Aleksey Baulin Cc: Thomas Monjalon , dev@dpdk.org, "Wiles, Keith" Message-ID: <20180114091709.28a31b4d@xeon-e3> In-Reply-To: References: <1511129764-23123-1-git-send-email-Aleksey.Baulin@gmail.com> <5180253.XvhpJrJZVt@xps> <216561584.1QX0fu7NSy@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Sun, 14 Jan 2018 17:17:18 -0000 On Sun, 14 Jan 2018 01:45:42 +0300 Aleksey Baulin wrote: > Please see my comments inline. >=20 > On Sun, Jan 14, 2018 at 1:24 AM, Thomas Monjalon > wrote: >=20 > > Hi, > > > > I moved your top-post below and did some comments inline. > > More opinions are welcome. > > > > 13/01/2018 23:05, Aleksey Baulin: =20 > > > On Fri, Jan 12, 2018 at 6:35 PM, Thomas Monjalon > > > wrote: =20 > > > > 21/11/2017 08:05, Aleksey Baulin: =20 > > > > > On Mon, Nov 20, 2017 at 4:36 PM, Wiles, Keith > > =20 > > > > wrote: =20 > > > > > > > On Nov 19, 2017, at 4:16 PM, Aleksey Baulin < =20 > > > > aleksey.baulin@gmail.com> =20 > > > > > > wrote: =20 > > > > > > > -#define unlikely(x) __builtin_expect((x),0) > > > > > > > +#define unlikely(x) __builtin_expect(!!(x), 0) =20 > > > > > > > > > > > > I have not looked at the generated code, but does this add some= =20 > > extra =20 > > > > > > instruction now to do the !!(x) ? =20 > > > > > > > > > > Sorry for late response. Jim had given the correct answer already. > > > > > You won't get an extra instruction with compiler optimization tur= ned =20 > > on. =20 > > > > > > > > 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 !=3D NULL) looks better than likely(pointer). =20 > > > > > > This is an interesting question. Perhaps, even a philosophical one. := -) > > > > > > 'likely(pointer)' is a perfectly legal statement in C language, as we= ll =20 > > as =20 > > > a concise one as > > > compared to a more explicit (and redundant/superfluous) 'likely(point= er =20 > > !=3D =20 > > > NULL)'. If you > > > _require_ this kind of explicitness in cases like this in the code st= yle, > > > then I have no > > > argument here. However, I don't see that anywhere. =20 > > > > It is stated here: > > http://dpdk.org/doc/guides/contributing/coding_style. > > html#null-pointers =20 >=20 >=20 > =E2=80=8BOh, thanks for pointing that out! I am sincerely ashamed for mis= sing it.=E2=80=8B > I lose that argument as I certainly do submit to the coding style. My only > excuse is that I am actually developing an app and not the DPDK core. >=20 >=20 > > > 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 ca= ses =20 > > on =20 > > > 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 DP= DK =20 > > is =20 > > > built. I write in C > > > for many years with the most time spent in kernels, Linux and not, an= d I > > > find it unnatural to > > > always add a redundant '!=3D NULL' just to satisfy the current macro > > > implementation. I would > > > have to accept that though if it's a requirement clearly stated somew= here > > > like a code style. > > > > > > As for an extra instruction, I believe that everything in DPDK =20 > > distribution =20 > > > is compiled with > > > optimization. So the execution speed in not a concern here. Perhaps t= here > > > are cases where > > > it's compiled without optimization, like debugging, but then I believ= e =20 > > it's =20 > > > a non-issue. =20 > > > > Yes you're right about optimization. > > But can we be 100% sure that it is always well optimized? > > =20 >=20 > =E2=80=8BI believe we can. I hope we get other opinions as well.=E2=80=8B >=20 > > 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. > > =20 >=20 > =E2=80=8BYes, that's exactly my case. Thanks.=E2=80=8B >=20 My opinion is that the DPDK likely() macro must behave exactly the same as the kernel and other projects. Doing something unique is not a great ben= efit.