[FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 21:42 Uhr schrieb James Almer <jamrial at gmail.com>:
>
> On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer <jamrial at gmail.com>:
> >>
> >> On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> >>> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer <jamrial at gmail.com>:
> >>>>
> >>>> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> >>>>> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis <dalecurtis at chromium.org>:
> >>>>>>
> >>>>>> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
> >>>>>> ---
> >>>>>>  libavutil/common.h | 10 ++++++++++
> >>>>>>  1 file changed, 10 insertions(+)
> >>>>>
> >>>>> Imo, this is guaranteed to break x86 compilation with some unusual
> >>>>> toolchain.
> >>>>> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> >>>>> and I believe they are in fact different (not for x86 or combined with other
> >>>>> checks).
> >>>>
> >>>> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
> >>>> used in public headers to choose one or another path depending on if the
> >>>> compiler is a recent enough GCC, or something else.
> >>>
> >>>> What toolchain could this break
> >>>
> >>> It definitely breaks icc compilation on Linux.
> >>
> >> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
> >
> > No, but yes, this is the effect.
>
> So that means ICC is broken, defining support for features it doesn't
> really has.
> We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
> but perhaps we may want to stop supporting broken compilers.

Or we add the necessary two lines of configure code necessary to avoid
the issue.

> >>>> and why specifically x86?
> >>>
> >>> I mentioned x86 because afaics, all existing relevant uses of
> >>> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
> >>
> >> AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
> >> headers, and in x86/intmath.h
> >
> > This is exactly what I meant above with "carefully".
>
> It would be nice if you could stop being so terse and explain what's on
> your mind for once. You're not being clear at all, and i asked you to
> elaborate, something you still haven't done.

To make this crystal-clear:
It would be nice if you could stop writing offending and annoying mails as
you did today.

The intmath.h check is surrounded by an additional check that looks much
stricter to me than the AV_GCC_VERSION_AT_LEAST() check there.

Carl Eugen