-
Notifications
You must be signed in to change notification settings - Fork 630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support link against libatomic if no built-in exist #212
Conversation
a16a261
to
ad94770
Compare
bldsys/cmake/check_atomic.cmake
Outdated
# check 64-bit atomics work without the library. | ||
check_working_cxx_atomics64(HAVE_CXX_ATOMICS64_WITHOUT_LIB) | ||
if (NOT HAVE_CXX_ATOMICS_WITHOUT_LIB OR NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB) | ||
set(NEED_LINK_ATOMIC ON CACHE BOOL "weither need to link to atomic library" FORCE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a slight typo in here. It should read "wether" instead of "weither".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually "whether" :)
Guidance for reviewers: Note, this PR is specific to MIPS and PowerPC platforms, so we mainly need folks to review the proposed changes and to ensure this won't disrupt anything on other platforms. Thanks! |
@sandy-lcq, can you check the copywrite on CMakelists.text? It's showing 2019 which breaks the copywrite headers check. Thanks. |
some arch like mips/ppc don't have built-in atomic, which will cause below build error: | framework/lib/ppc/libframework.a(device.cpp.o): in function `std::__atomic_base<unsigned long long>::load(std::memory_order) const': | /usr/include/c++/10.2.0/bits/atomic_base.h:426: undefined reference to `__atomic_load_8' Signed-off-by: Changqing Li <changqing.li@windriver.com>
@ALL, Thanks for your comments. I have correct them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build works ok for Broadcom
Hi Team, we'd like to get 3 approvals so we can merge this one. Even though this is specific to MIPS and PowerPC we'd like to verify that it doesn't break on other platforms. Thanks! @r-potter, @aurolou, @JoseEmilio-ARM, @mattruschnv, @AlejandroCosin, @carlosadcq |
some arch like mips/ppc don't have built-in atomic, which will
cause below build error:
| framework/lib/ppc/libframework.a(device.cpp.o): in function
std::__atomic_base<unsigned long long>::load(std::memory_order) const': | /usr/include/c++/10.2.0/bits/atomic_base.h:426: undefined reference to
__atomic_load_8'Signed-off-by: Changqing Li changqing.li@windriver.com