Skip to content

Commit

Permalink
cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V
Browse files Browse the repository at this point in the history
The check for 'HAVE_CXX_ATOMICS_WITHOUT_LIB' may create false
positives in RISC-V. This is reproducible when compiling LLVM natively
using GCC on a rv64gc (rv64imafdgc) host. Due to the 'A' (atomic)
extension, g++ replaces calls to libatomic operations on the
std::atomic<int> type with the native hardware instructions. As a
result, the compilation succeeds and the build system thinks it
doesn't need to pass '-latomic'.

To complicate matters, even when the 'A' extension is forcibly
disabled (by passing '-march=rv64imfd'), g++ compiles the test code
snippet just fine.

To the contrary, '__atomic' builtins or C++11 atomics requires linking
against '-latomic' in RISC-V (See
riscvarchive/riscv-gcc#12).

This causes the following build failure for dsymutil:

: && /usr/bin/riscv64-unknown-linux-gnu-g++  -O2 -pipe -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections  -Wl,-O1 -Wl,--as-needed -Wl,-allow-shlib-undefined    -Wl,-rpath-link,/var/tmp/portage/sys-devel/llvm-10.0.0.9999/work/llvm-10.0.0.9999-abi_riscv_lp64d.lp64d/./lib64/lp64d  -Wl,-O3 -Wl,--gc-sections tools/dsymutil/CMakeFiles/dsymutil.dir/dsymutil.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/BinaryHolder.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/CFBundle.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/CompileUnit.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/DeclContext.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/DwarfLinker.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/DwarfStreamer.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/MachODebugMapParser.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/MachOUtils.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/NonRelocatableStringpool.cpp.o tools/dsymutil/CMakeFiles/dsymutil.dir/SymbolMap.cpp.o  -o bin/dsymutil  -Wl,-rpath,"\$ORIGIN/../lib64/lp64d" lib64/lp64d/libLLVM-10svn.so -lpthread && :
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: tools/dsymutil/CMakeFiles/dsymutil.dir/dsymutil.cpp.o: in function `.LEHB79':
dsymutil.cpp:(.text._ZNSt17_Function_handlerIFvvESt5_BindIFZ4mainEUlSt10shared_ptrIN4llvm14raw_fd_ostreamEENS3_8dsymutil11LinkOptionsEE_S5_S7_EEE9_M_invokeERKSt9_Any_data+0x168): undefined reference to `__atomic_fetch_and_1'
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: tools/dsymutil/CMakeFiles/dsymutil.dir/dsymutil.cpp.o: in function `.L0 ':
dsymutil.cpp:(.text._ZNSt17_Function_handlerIFvvESt5_BindIFZ4mainEUlSt10shared_ptrIN4llvm14raw_fd_ostreamEENS3_8dsymutil11LinkOptionsEE_S5_S7_EEE9_M_invokeERKSt9_Any_data+0x2ee): undefined reference to `__atomic_fetch_and_1'
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: dsymutil.cpp:(.text.startup.main+0xefc): undefined reference to `__atomic_fetch_and_1'
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: tools/dsymutil/CMakeFiles/dsymutil.dir/dsymutil.cpp.o: in function `.L2508':
dsymutil.cpp:(.text.startup.main+0x1b2a): undefined reference to `__atomic_fetch_and_1'
collect2: error: ld returned 1 exit status

This is likely because dsymutil is defining a variable of type
'std::atomic_char' (See:
https://github.com/llvm/llvm-project/blob/release/9.x/llvm/tools/dsymutil/dsymutil.cpp#L542)

Improve the reliability of the 'HAVE_CXX_ATOMICS_WITHOUT_LIB' test in
two steps:

1. Force a pre-increment on x (++x), which should force a call to a
libatomic function per the prototype:

  __int_type
  operator++(int) volatile noexcept
  { return fetch_add(1); }

2. Because step 1 would resolve the increment to 'amoadd.w.aq' under
the 'A' extension, force the same operation on sub-word types, for
which there is no hardware support. This effectively forces g++ to
insert calls to libatomic functions.

Differential Revision: https://reviews.llvm.org/D68964

Signed-off-by: Gokturk Yuksek <[email protected]>
  • Loading branch information
gktrk committed Oct 25, 2019
1 parent 074af2d commit d7c519e
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion llvm/cmake/modules/CheckAtomic.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ function(check_working_cxx_atomics varname)
CHECK_CXX_SOURCE_COMPILES("
#include <atomic>
std::atomic<int> x;
std::atomic<short> y;
std::atomic<char> z;
int main() {
return x;
++z;
++y;
return ++x;
}
" ${varname})
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
Expand Down

0 comments on commit d7c519e

Please sign in to comment.