From: Aleksey Baulin <Aleksey.Baulin@gmail.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, "Wiles, Keith" <keith.wiles@intel.com>
Subject: Re: [dpdk-dev] [PATCH] eal/common: better likely() and unlikely()
Date: Sun, 14 Jan 2018 01:05:14 +0300 [thread overview]
Message-ID: <CAJ+OjZn6Zw7y2Wj4FSoWxn48DeCTrObSfPxE0gA+mXPXeOu12Q@mail.gmail.com> (raw)
In-Reply-To: <5180253.XvhpJrJZVt@xps>
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.
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.
Hope my explanations shed some more light on this patch. :-)
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.com>
> 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).
>
--
Aleksey Baulin
next prev parent reply other threads:[~2018-01-13 22:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-19 22:16 Aleksey Baulin
2017-11-20 13:36 ` Wiles, Keith
2017-11-20 17:21 ` Jim Thompson
2017-11-21 7:05 ` Aleksey Baulin
2018-01-12 15:35 ` Thomas Monjalon
2018-01-13 22:05 ` Aleksey Baulin [this message]
2018-01-13 22:24 ` Thomas Monjalon
2018-01-13 22:45 ` Aleksey Baulin
2018-01-14 17:17 ` Stephen Hemminger
2018-01-20 16:28 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ+OjZn6Zw7y2Wj4FSoWxn48DeCTrObSfPxE0gA+mXPXeOu12Q@mail.gmail.com \
--to=aleksey.baulin@gmail.com \
--cc=dev@dpdk.org \
--cc=keith.wiles@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).