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

Ganesh Ajjanagadde gajjanag at gmail.com
Sat Feb 27 14:11:40 CET 2016


On Thu, Feb 25, 2016 at 8:55 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 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.

I mostly agree with the sentiments. Patchset shelved until a
non-trivial, critical use case can be found.

@Matt: feel free to extend to ICC if/when this happens; I lack it and
can't test.

Thanks all.

>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list