[libcamera-devel] [PATCH v3 4/7] libcamera: Don't unnecessarily link against libatomic

[Kieran Bingham]

Hi Laurent,

On 07/03/2020 21:13, Laurent Pinchart wrote:
> libatomic provides atomic operations that are also available as compiler
> built-ins on most platforms. On some platforms (such as Sparc) linking
> against libatomic is required to use any atomic operation. libcamera
> links against libatomic if available for that reason.
> 
> This however makes libcamera link against libatomic even when not
> required, if libatomic is available. Restrict this to only the cases
> where libatomic is required with a link test.
> 

Hrm, I thought we discussed and researched this topic when it went in.

I thought I recalled testing this and identifying that even if libatomic
was found on the system, and would be available to link - The toolchain
would take the internals as a preference and caused no binary change in
the output.

But I won't overly object to this change though.

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  meson.build               |  1 +
>  src/libcamera/meson.build |  2 +-
>  src/meson.build           | 17 +++++++++++++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index c6e6a934e54e..a1e455249b0d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -24,6 +24,7 @@ libcamera_version = libcamera_git_version.split('+')[0]
>  , 
>  # Configure the build environment.
>  cc = meson.get_compiler('c')
> +cxx = meson.get_compiler('cpp')
>  config_h = configuration_data()
>  
>  if cc.has_header_symbol('execinfo.h', 'backtrace')
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 1b69bc0dee54..ac6f597c6188 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -93,7 +93,7 @@ version_cpp = vcs_tag(command : [gen_version, meson.build_root()],
>  libcamera_sources += version_cpp
>  
>  libcamera_deps = [
> -    cc.find_library('atomic', required: false),
> +    libatomic_dep,
>      cc.find_library('dl'),
>      libudev,
>      dependency('threads'),
> diff --git a/src/meson.build b/src/meson.build
> index d818d8b86d93..abf755936f2b 100644
> --- a/src/meson.build
> +++ b/src/meson.build

Hrm ... is this the right place for this?

I guess it's not a 'bad' place for it.
Our top level meson.build is getting busier and busier... and this
presumably affects only 'src' code (hence being under src) ... but then
'test/' misses out?

Still I guess test/ may be less likely to use atomics...


> @@ -1,3 +1,20 @@
> +# On some platforms gcc doesn't provide built-ins for atomic operations,
> +# explicit linking against libatomic is required.
> +if cxx.links('''
> +    #include <atomic>
> +    #include <cstdint>
> +    int main(void)
> +    {
> +        std::atomic<std::uint32_t> u32(0);
> +        std::atomic<std::uint64_t> u64(0);
> +        return u32.load() + u64.load();
> +    }
> +''')
> +    libatomic_dep = dependency('', required : false)
> +else
> +    libatomic_dep = cc.find_library('atomic')
> +endif
> +
>  if get_option('android')
>      subdir('android')
>  endif
> 

-- 
Regards
--
Kieran