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