[dpdk-dev] [PATCH] eal/common: better likely() and unlikely()

Aleksey Baulin Aleksey.Baulin at gmail.com
Sat Jan 13 23:05:14 CET 2018


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 at monjalon.net>
wrote:

> 21/11/2017 08:05, Aleksey Baulin:
> > On Mon, Nov 20, 2017 at 4:36 PM, Wiles, Keith <keith.wiles at intel.com>
> wrote:
> > > > On Nov 19, 2017, at 4:16 PM, Aleksey Baulin <
> aleksey.baulin at 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


More information about the dev mailing list