[FFmpeg-devel] [PATCH 1/3] lavu/attributes: introduce av_likely, av_unlikely

[Hendrik Leppkes]

On Thu, Feb 25, 2016 at 8:27 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Thu, 25 Feb 2016 15:48:17 +1100
> Matt Oliver <protogonoi at gmail.com> wrote:
>
>> On 25 February 2016 at 13:20, Ganesh Ajjanagadde <gajjanag at gmail.com> wrote:
>>
>> > From: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >
>> > These use __builtin_expect, and may be useful for optimizing nearly
>> > certain branches, to yield size and/or speed improvements.
>> >
>> > Note that this is used in the Linux kernel for the same purpose. For
>> > some idea as to potential benefits, see e.g
>> > http://blog.man7.org/2012/10/how-much-do-builtinexpect-likely-and.html.
>> >
>> > Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> > ---
>> >  libavutil/attributes.h | 8 ++++++++
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/libavutil/attributes.h b/libavutil/attributes.h
>> > index 5c6b9de..1547033 100644
>> > --- a/libavutil/attributes.h
>> > +++ b/libavutil/attributes.h
>> > @@ -58,6 +58,14 @@
>> >  #    define av_warn_unused_result
>> >  #endif
>> >
>> > +#if AV_GCC_VERSION_AT_LEAST(3,0)
>> > +#    define av_likely(x) __builtin_expect(!!(x), 1)
>> > +#    define av_unlikely(x) __builtin_expect(!!(x), 0)
>> > +#else
>> > +#    define av_likely(x) (x)
>> > +#    define av_unlikely(x) (x)
>> > +#endif
>> > +
>> >  #if AV_GCC_VERSION_AT_LEAST(3,1)
>> >  #    define av_noinline __attribute__((noinline))
>> >  #elif defined(_MSC_VER)
>> >
>>
>> Ive used these builtins before and can confirm that they are useful
>> (although requires devs to use them obviously). I will also point out that
>> these are also supported by ICC/ICL as well.
>
> They also make code ugly as fuck, and are more cargo-cult than anything
> else. They might possibly be ok in some very critical parts of the
> code, but otherwise not at all.
>

I agree with wm4 on the ugly aspect, I always hate reading code from
projects that use it, its terrible on readability.
If its used at all, it should be proven that (a) the function is
performance relevant, and (b) that the improvement is actually
tangible in the grand scheme.

- Hendrik