Re: [FFmpeg-devel] [PATCH] configure: link to libatomic when it's present

[James Almer <jamrial@gmail.com>]

On 1/19/2022 9:40 AM, Anton Khirnov wrote:
> Quoting James Almer (2022-01-19 13:34:20)
>>
>>
>> On 1/19/2022 8:54 AM, Anton Khirnov wrote:
>>> C11 atomics in some configurations (e.g. 64bit operations on ppc64 with
>>> GCC) require linking to libatomic.
>>> ---
>>> Testing welcome, especially in configurations where
>>> * libatomic is not present
>>> * libatomic is actually needed
>>> ---
>>>    configure | 9 ++++++++-
>>>    1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 1413122d87..1ff5dbee5b 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -6324,7 +6324,14 @@ check_headers asm/types.h
>>>    # it seems there are versions of clang in some distros that try to use the
>>>    # gcc headers, which explodes for stdatomic
>>>    # so we also check that atomics actually work here
>>> -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar"
>>> +#
>>> +# some configurations also require linking to libatomic, so try
>>> +# both with -latomic and without
>>> +for LATOMIC in "-latomic" ""; do
>>
>> Shouldn't you try without it first? On my toolchain libatomic is
>> present, but libraries compile without linking to it just fine. That
>> changes after this patch, where it starts linking to it explicitly.
> 
> No, because it may only be needed for some atomic sizes and operations,
> which the test in configure doesn't necessarily catch.
> And because we pass as-needed to the linker, the built libraries
> shouldn't actually require libatomic unless it's really needed.

Ah, didn't consider --as-needed.

> 
>>
>>> +    check_builtin stdatomic stdatomic.h                                                 \
>>> +        "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar"  \
>>> +        $LATOMIC && add_extralibs $LATOMIC && break
>>
>> You should probably add it to the required libraries' extralibs only.
>> Just replace the add_extralibs part with setting stdatomic_extralibs to
>> $LATOMIC, and then add stdatomic to all the libraries' _suggest lists,
>> same as we do for libm.
> 
> Then we need to actively track which libraries actually use atomics,
> which also depends on which features are enabled. Given that this only
> fails on less-common arches, this sounds like a recipe for obscure build
> failures. Given that it's only really linked when needed, it seems
> better to just add it unconditionally.

Then just add it to all of them, like we do for libm.

What i want to avoid is having it in EXTRALIBS. There was a huge 
configure rework long ago that removed everything from that variable 
(leaving it as the place where user defined --extra-libs arguments are 
dumped), and fine tuned ld arguments in a per library/module basis. I'd 
like to not go back to start dumping everything in EXTRALIBS.

Like this, on top of this patch:

> diff --git a/configure b/configure
> index 1ff5dbee5b..43713a7679 100755
> --- a/configure
> +++ b/configure
> @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs"
> 
>  # libraries, in any order
>  avcodec_deps="avutil"
> -avcodec_suggest="libm"
> +avcodec_suggest="libm stdatomic"
>  avdevice_deps="avformat avcodec avutil"
> -avdevice_suggest="libm"
> +avdevice_suggest="libm stdatomic"
>  avfilter_deps="avutil"
> -avfilter_suggest="libm"
> +avfilter_suggest="libm stdatomic"
>  avformat_deps="avcodec avutil"
> -avformat_suggest="libm network zlib"
> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt"
> +avformat_suggest="libm network stdatomic zlib"
> +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl stdatomic user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt"
>  postproc_deps="avutil gpl"
> -postproc_suggest="libm"
> +postproc_suggest="libm stdatomic"
>  swresample_deps="avutil"
> -swresample_suggest="libm libsoxr"
> +swresample_suggest="libm libsoxr stdatomic"
>  swscale_deps="avutil"
> -swscale_suggest="libm"
> +swscale_suggest="libm stdatomic"
> 
>  avcodec_extralibs="pthreads_extralibs iconv_extralibs dxva2_extralibs"
>  avfilter_extralibs="pthreads_extralibs"
> @@ -6330,7 +6330,7 @@ check_headers asm/types.h
>  for LATOMIC in "-latomic" ""; do
>      check_builtin stdatomic stdatomic.h                                                 \
>          "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar"  \
> -        $LATOMIC && add_extralibs $LATOMIC && break
> +        $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break
>  done
> 
>  check_lib advapi32 "windows.h"            RegCloseKey          -ladvapi32
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".