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