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

Marton Balint cus at passwd.hu
Thu Jan 24 23:45:00 EET 2019


On Thu, 24 Jan 2019, 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.

Well, the reason behind it is patch 3/3 where it matters accordig to my 
tests. I wonder if it is only my compiler version where it makes a 
difference.

Regards,
Marton


More information about the ffmpeg-devel mailing list