[cmake] link atomic library for certain CPU architectures #21743
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
https://bugs.gentoo.org/864421 |
wsnipex
reviewed
Aug 10, 2022
CMakeLists.txt
Outdated
| @@ -127,6 +127,9 @@ set(THREADS_PREFER_PTHREAD_FLAG ON) | |||
| find_package(Threads REQUIRED QUIET) | |||
| list(APPEND DEPLIBS ${CMAKE_THREAD_LIBS_INIT}) | |||
|
|
|||
| # Atomics | |||
| find_package(Atomics REQUIRED) | |||
There was a problem hiding this comment.
since this is linux and riscv/arm specific, this is not globally required. Instead move detection to https://github.com/xbmc/xbmc/blob/master/cmake/scripts/linux/ArchSetup.cmake and add atomics to PLATFORM_REQUIRED_DEPS only when needed
For those CPU architectures: RISC-V lack 8-bit and 16-bit atomic instructions, and ARM/MIPS/PPC lack 64-bit atomic instruction. GCC is supposed to convert these atomics via masking and shifting like LLVM, which means anything that wants to use these instructions needs the link option -latomic. In this patch, we will try to detect if 8-bit, 64-bit atomic instructions exist, otherwise the atomic library will append to the DEPLIBS list. Original issue: * https://gitlab.kitware.com/cmake/cmake/-/issues/23021#note_1098733 For reference: * https://gcc.gnu.org/wiki/Atomic/GCCMM riscv64 specific: * https://lists.debian.org/debian-riscv/2022/01/msg00009.html Signed-off-by: Yixun Lan <dlan@gentoo.org>
|
@wsnipex hi, how about this new version? is it ok to merge? or, I'd really appreciate if you can give some comments |
lrusak
added
CMake
Platform: Linux
Type: Fix
|
@wsnipex can you please review this updated PR? Thank you! |
gentoo-bot
pushed a commit
to gentoo/gentoo
that referenced
this pull request
Aug 26, 2022
Upstream: xbmc/xbmc#21743 Closes: https://bugs.gentoo.org/864421 Signed-off-by: Yixun Lan <dlan@gentoo.org> Closes: #27016 Signed-off-by: Jakov Smolić <jsmolic@gentoo.org>
wsnipex
approved these changes
Sep 5, 2022
|
Thanks for the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For those CPU architectures:
RISC-V lack 8-bit and 16-bit atomic instructions, and
ARM/MIPS/PPC lack 64-bit atomic instruction.
GCC is supposed to convert these atomics via masking and shifting
like LLVM, which means anything that wants to use these instructions
needs the link option -latomic.
In this patch, we will try to detect if 8-bit, 64-bit atomic instructions exist,
otherwise the atomic library will append to the DEPLIBS list.
Signed-off-by: Yixun Lan dlan@gentoo.org
Description
Motivation and context
How has this been tested?
What is the effect on users?
Screenshots (if appropriate):
Types of change
Checklist: