Fix detection of C11 atomics #68
Conversation
Currently, the CMakeLists.txt logic to detect the availability of C11 atomics is based on building a small program that uses the atomic_load(). However, atomic_load() does not need to use any function from libatomic (part of the gcc runtime). So even if libatomic is missing, this test concludes that C11 atomic support is available. For example on SPARC, the example program builds fine without linking to libatomic, but calling other functions of the atomic_*() APIs fail without linking to libatomic. So, this patch adjusts the CMakeLists.txt test to use a function that is known to require the libatomic run-time library (on architectures where it is needed). This way, openal will only use the __atomic_*() built-ins when they are actually functional. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
|
This will actually break Clang, or certain versions of it at least, which support C11 atomics but fail on things like |
|
Actually, I'm curious about the consequences of using libatomic. In normal cases, atomic methods should generate inline assembly rather than external function calls. If it's calling into a separate library, that makes me concerned that the functions are using slow mutex-protected implementations. Do both C11 atomics and GCC's built-in atomics both call into external functions? If so, why doesn't GCC's built-ins use the same lib as C11 atomics? |
|
@kcat There are platforms that really don't provide the appropriate atomic instructions, so there is no other choice than to have a slower mutex-based implementation. The code in libatomic is only used if there are no way around it. |
|
Okay, that makes sense. Commit a004ccf adds a check for, and links with, libatomic, so C11 atomics should work now. It'll also check |
|
@kcat Unfortunately, this doesn't work: it doesn't detect that atomic_fetch_add is available in libatomic (tested on SPARC, where all atomic_*() functions are provided in libatomic) : From the CMake error log: I am not sure how CMake is doing its function test, but atomic_fetch_add() is not a normal function, it gets replaced by a call to atomic_fetch_add_4, atomic_fetch_add_8, atomic_fetch_add_2, atomic_fetch_add_1 depending on the size of the integer passed as argument. |
|
I see. This is turning out to be unfortunate... I got the check working using a different method, but there's another issue. If libatomic exists in the build environment, it'll be linked. Even if it's not needed on the target, as long as it's included in the link command it'll be a(n unnecessary) dependency. Checking if it's actually needed isn't so simple either, since different CPU targets will have different needs (some may need external functions for incrementing/decrementing integers by more than one, some may need it for 8-byte types, some may need it for compare-exchange..). |
|
Commit af5fb3d should properly detect libatomic. I'll worry about linking it only when needed later. Does it work now? (make sure to do a fresh build, or clear |
|
Closing since this should be handled now. It links with libatomic if it's there, regardless if it's really needed or not. If there's still a problem, please open a new Issue or make a new PR. |
Currently, the CMakeLists.txt logic to detect the availability of C11
atomics is based on building a small program that uses the
atomic_load().
However, atomic_load() does not need to use any function from
libatomic (part of the gcc runtime). So even if libatomic is missing,
this test concludes that C11 atomic support is available. For example
on SPARC, the example program builds fine without linking to
libatomic, but calling other functions of the atomic_*() APIs fail
without linking to libatomic.
So, this patch adjusts the CMakeLists.txt test to use a function that
is known to require the libatomic run-time library (on architectures
where it is needed). This way, openal will only use the _atomic*()
built-ins when they are actually functional.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com