Re: [igt-dev] [PATCH i-g-t 1/4] tests: add libatomic dependency

Guillaume Tucker
2019-06-13T12:55:35Z

On 06/06/2019 08:18, Ser, Simon wrote:
> On Mon, 2019-06-03 at 12:54 +0100, Guillaume Tucker wrote:
>> Add dependency to libatomic in order to be able to use the __atomic_*
>> functions instead of the older __sync_* ones. This is to enable
>> atomic operations on a wider number of architectures including MIPS.
>
> Thanks for your patch! I have a few questions because I don't know well
> how libatomic works.

Thanks for the review!

> Do we want to always link against libatomic? For instance LLVM tries to
> compile a program with atomic before falling back to libatomic:
> https://github.com/llvm-mirror/llvm/blob/master/cmake/modules/CheckAtomic.cmake
>
> Should this dependency be mandatory?

I've had a look around, the short answer is we should make this
dependency optional. I'm sending a v2 of this series which
addresses this issue.

From what I understand, linking against libatomic is actually
only needed when some atomic operations aren't supported natively
by a CPU architecture. For example, this is the case with 64-bit
atomics on 32-bit MIPS. If the CPU can't do an atomic operation
and there's no libatomic available, then it won't build. So I've
made a test in meson.build to check whether linking against
libatomic is required or not, to drop the dependency when it
isn't. I've verified that the test passes on x86 (i.e. no
dependency) and fails on 32-bit MIPS (i.e. with dependency).

Guillaume

>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>> meson.build | 1 +
>> tests/meson.build | 2 +-
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 6268c58d3634..4e5bb323fa49 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -179,6 +179,7 @@ math = cc.find_library('m')
>> realtime = cc.find_library('rt')
>> dlsym = cc.find_library('dl')
>> zlib = cc.find_library('z')
>> +libatomic = cc.find_library('atomic')
>>
>> if cc.has_header('linux/kd.h')
>> config.set('HAVE_LINUX_KD_H', 1)
>> diff --git a/tests/meson.build b/tests/meson.build
>> index 806766e51667..6877ccd59235 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -233,7 +233,7 @@ i915_progs = [
>> 'i915_suspend',
>> ]
>>
>> -test_deps = [ igt_deps ]
>> +test_deps = [ igt_deps, libatomic ]
>>
>> if libdrm_nouveau.found()
>> test_progs += [