[FFmpeg-devel] [PATCH 1/3] avutil/attributes: add av_likely and av_unlikely

[Michael Niedermayer]

On Thu, Jan 24, 2019 at 06:08:57PM -0300, James Almer wrote:
> On 1/24/2019 5:53 PM, Rostislav Pehlivanov wrote:
> > On Thu, 24 Jan 2019 at 20:38, Marton Balint <cus at passwd.hu> wrote:
> > 
> >> Signed-off-by: Marton Balint <cus at passwd.hu>
> >> ---
> >>  doc/APIchanges         | 3 +++
> >>  libavutil/attributes.h | 8 ++++++++
> >>  libavutil/version.h    | 2 +-
> >>  3 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index a39a3ff2ba..38b5b980a6 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> >>
> >>  API changes, most recent first:
> >>
> >> +2019-01-xx - xxxxxxxxxx - lavu 56.27.100 - attributes.h
> >> +  Add av_likely and av_unlikely
> >> +
> >>  2019-01-08 - xxxxxxxxxx - lavu 56.26.100 - frame.h
> >>    Add AV_FRAME_DATA_REGIONS_OF_INTEREST
> >>
> >> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> >> index ced108aa2c..60972e5109 100644
> >> --- a/libavutil/attributes.h
> >> +++ b/libavutil/attributes.h
> >> @@ -164,4 +164,12 @@
> >>  #    define av_noreturn
> >>  #endif
> >>
> >> +#if AV_GCC_VERSION_AT_LEAST(2,96) || defined(__clang__)
> >> +# 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
> >> +
> >>  #endif /* AVUTIL_ATTRIBUTES_H */
> >> diff --git a/libavutil/version.h b/libavutil/version.h
> >> index 1fcdea95bf..12b4f9fc3a 100644
> >> --- a/libavutil/version.h
> >> +++ b/libavutil/version.h
> >> @@ -79,7 +79,7 @@
> >>   */
> >>
> >>  #define LIBAVUTIL_VERSION_MAJOR  56
> >> -#define LIBAVUTIL_VERSION_MINOR  26
> >> +#define LIBAVUTIL_VERSION_MINOR  27
> >>  #define LIBAVUTIL_VERSION_MICRO 100
> >>
> >>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >> --
> >> 2.16.4
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > 
> > 
> > NAK, NAK, NAK.
> > I will NAK the hell out of any patch that does that. They're completely
> > unnecessary, they're commonly used by complete idiots who add them thinking
> > their code will go faster (and it might - only on their antiquated GCC
> > 3.1), they're religiously used by the same people and won't back down on
> > using them and finally they're ugly when added to every single bloody
> > branch and the people who maintain such code will demand they be added to
> > every single bloody branch for no reason other that what I've just iterated
> > on.
> > The ONLY way I'll accept them is in platform-specific files, e.g.
> > libavcodec/ppc/*, and even then only on non-x86 arches (which need them for
> > having bad compilers with primitive processors having awful branch
> > prediction) and even then I'd never accept their inclusioin into
> > system-installed headers.
> 
> What about hot loops with branches where you can use these as a hint for
> the compiler to assume a specific outcome is highly unlikely to happen,
> like alloc errors, corner cases in some codec, etc, which it simply has
> no way to correctly guess at compile time?
> 
> I don't think it should be NAKed because it's misused in other projects.
> We're not other projects. We should instead simply ask the patch writer
> to provide numbers that prove using it makes a difference in their code
> with a recent version of GCC/Clang, and if it's negligible or within the
> margin of error we'll simply ask to remove it to keep the code clean.

if we want to ensure this, it can be enforced easily
we have fate-source, that can check litterally for each av_(un)likely
to contain a comment on the same line listing a non negligible performance
effect. as in // branch hint increases speed by 5%

OTOH, it may make more sense to gather branch statistics at runtime and
include that in the next build without smudging the source

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190124/3d86eb37/attachment.sig>