Messages in this thread | | | Subject | Re: [PATCH v2] scripts: coccinelle: check for !(un)?likely usage | From | Rasmus Villemoes <> | Date | Fri, 30 Aug 2019 10:06:39 +0200 |
| |
On 30/08/2019 08.56, Denis Efremov wrote: > > > On 30.08.2019 03:42, Julia Lawall wrote: >> >> >> On Thu, 29 Aug 2019, Denis Efremov wrote: >> >>> On 8/29/19 8:10 PM, Denis Efremov wrote: >>>> This patch adds coccinelle script for detecting !likely and >>>> !unlikely usage. These notations are confusing. It's better >>>> to replace !likely(x) with unlikely(!x) and !unlikely(x) with >>>> likely(!x) for readability. >>> >>> I'm not sure that this rule deserves the acceptance. >>> Just to want to be sure that "!unlikely(x)" and "!likely(x)" >>> are hard-readable is not only my perception and that they >>> become more clear in form "likely(!x)" and "unlikely(!x)" too. >> >> Is likely/unlikely even useful for anything once it is a subexpression? >>> julia >> > > Well, as far as I understand it,
Yes, and it could in fact make sense in cases like
if (likely(foo->bar) && unlikely(foo->bar->baz)) { do_stuff_with(foo->bar->baz); do_more_stuff(); }
which the compiler could then compile as (of course actual code generation is always much more complicated due to things in the surrounding code)
load foo->bar; test bar; if 0 goto skip; load bar->baz; test baz; if !0 goto far_away; skip: ....
so in the normal flow, neither branch is taken. If instead one wrote unlikely(foo->bar && foo->bar->baz), gcc doesn't really know why we expect the whole conjuntion to turn out false, so it could compile this as a jump when foo->bar turns out non-zero - i.e., in the normal case, we'd end up jumping.
But as far as !(un)likely(), I agree that it's easier to read as a human if the ! operator is moved inside (and the "un" prefix stripped/added). Whether it deserves a cocci script I don't know.
Rasmus
| |