From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <keshonok@gmail.com> Received: from mail-ua0-f193.google.com (mail-ua0-f193.google.com [209.85.217.193]) by dpdk.org (Postfix) with ESMTP id B8023A490 for <dev@dpdk.org>; Sat, 13 Jan 2018 23:45:43 +0100 (CET) Received: by mail-ua0-f193.google.com with SMTP id a25so6356603uak.3 for <dev@dpdk.org>; Sat, 13 Jan 2018 14:45:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=h1UktZMjKVQJ8yp5XAntHO1I7CEKX7q7RnUSQpjnjqk=; b=QC0lPyr+JUIAgSfCFW/ATlSnO9b4s8DNxxYqXYAsHEYBMJuPdXj92QPE74o2Xnc1gf w3ks0840dY9fRgFUj5ZOsacljDlgj5g/Cfjd9IOzX29taUvKnKl+yY7xOx3iSR4ckZto Wb3q5g1YRjK+Q7sD6vwTa3A5hoSNSQ0OEf0bKcgO4zERz6lWCw1h6F5Ew0jchu3WbmYd c1aAaF3QaWLLw6ZJOWVoO16MdElcRlxED/8XqPErPfE3lxmXT0a/aJQtNane6k5Da1Oe vCYOTIo3kJefxLIBF7PgnxZdQQLrrdb4jbFmyNJN5W2ec4O/2jVKjoHHGAIb95Qz8Xti W4pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=h1UktZMjKVQJ8yp5XAntHO1I7CEKX7q7RnUSQpjnjqk=; b=akPfEewPpJnL4kweVOht8nlCap/BhP+zL/x/4FAs1zC7pvJuOpqGc9ej4FU/Gh9F6I 6pETrFT66LIzcL1lxqAjxlSEQzGuFqRCoh53tXU1Dv5/ps3zO5yk2yC6BonpjrwiD7bm 0HVDTZ2CKsV8Os+EYsHr3iK8fS6t0a0/aC8UNZOR8/3dn3tNbN5zX14j/2uVL6FWIdX7 l2rxQ3ufRw5GU413EhkGv2uetXyUbmdk5qBW6Y7xWijo1Cz56pDYJYVuyTSkhLypzzNq Vx8VfgzaP7LwgwaTGuvofBO8P3Qe4YUXI0hyGm9Zmgtm3EAfySoTyz17zfOyBy7etV5a bxMw== X-Gm-Message-State: AKwxytd/Uz2s/23yv+89PSCUQT+MOhf5nHT/IBxHzNRh/p+8cT2APnC1 EJz2LQWm5V0swP4E5Mf8EwPX0vXmpAkp9oy2DfA= X-Google-Smtp-Source: ACJfBosnEfuRbsIrL/eunmQdDumyUk1iFgD9d95a2t8Bkv4SfSU7LdPHo2Zw9JFn2aW0g/KOJEZMjqwWJXTAPDG20h4= X-Received: by 10.176.78.147 with SMTP id l19mr23112428uah.96.1515883543046; Sat, 13 Jan 2018 14:45:43 -0800 (PST) MIME-Version: 1.0 Sender: keshonok@gmail.com Received: by 10.31.166.148 with HTTP; Sat, 13 Jan 2018 14:45:42 -0800 (PST) In-Reply-To: <216561584.1QX0fu7NSy@xps> References: <1511129764-23123-1-git-send-email-Aleksey.Baulin@gmail.com> <5180253.XvhpJrJZVt@xps> <CAJ+OjZn6Zw7y2Wj4FSoWxn48DeCTrObSfPxE0gA+mXPXeOu12Q@mail.gmail.com> <216561584.1QX0fu7NSy@xps> From: Aleksey Baulin <Aleksey.Baulin@gmail.com> Date: Sun, 14 Jan 2018 01:45:42 +0300 X-Google-Sender-Auth: ppwuBGAJB3_JFJ71Q0IjyBH_WuY Message-ID: <CAJ+OjZmDyobz+kvaYtOMqGNC3K+h6DAuFCqGK9VpwR84fk7TZg@mail.gmail.com> To: Thomas Monjalon <thomas@monjalon.net> Cc: dev@dpdk.org, "Wiles, Keith" <keith.wiles@intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 <dev.dpdk.org> List-Unsubscribe: <https://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Sat, 13 Jan 2018 22:45:43 -0000 Please see my comments inline. On Sun, Jan 14, 2018 at 1:24 AM, Thomas Monjalon <thomas@monjalon.net> wrote: > 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 <thomas@monjalon.net> > > wrote: > > > 21/11/2017 08:05, Aleksey Baulin: > > > > On Mon, Nov 20, 2017 at 4:36 PM, Wiles, Keith <keith.wiles@intel.co= m > > > > > 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 turne= d > 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 !=3D 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 > !=3D > > NULL)'. If you > > _require_ this kind of explicitness in cases like this in the code styl= e, > > 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 =E2=80=8BOh, thanks for pointing that out! I am sincerely ashamed for missi= ng 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. > > 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 case= s > 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, th= ey > > 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 '!=3D NULL' just to satisfy the current macro > > implementation. I would > > have to accept that though if it's a requirement clearly stated somewhe= re > > 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 the= re > > 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? > =E2=80=8BI believe we can. I hope we get other opinions as well.=E2=80=8B > 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. > =E2=80=8BYes, that's exactly my case. Thanks.=E2=80=8B --=20 Aleksey Baulin