Hi, On Sat, Feb 27, 2016 at 8:11 AM, Ganesh Ajjanagadde <gajja...@gmail.com> wrote:
> On Thu, Feb 25, 2016 at 8:55 AM, Hendrik Leppkes <h.lepp...@gmail.com> > wrote: > > On Thu, Feb 25, 2016 at 8:27 AM, wm4 <nfx...@googlemail.com> wrote: > >> On Thu, 25 Feb 2016 15:48:17 +1100 > >> Matt Oliver <protogo...@gmail.com> wrote: > >> > >>> On 25 February 2016 at 13:20, Ganesh Ajjanagadde <gajja...@gmail.com> > wrote: > >>> > >>> > From: Ganesh Ajjanagadde <gajjanaga...@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 <gajjanaga...@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. I can think of a few. E.g. buffer overflow protection in bitstream reader (get_bits.h), maybe the arith readers (vp56.h, cabac.h) could also use these. These codepaths are actually quite performance sensitive and I'm not sure they're all running hand-coded assembly. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel