[PATCH] D69869: [clang-tools-extra] fix the check for if '-latomic' is necessary
Gokturk Yuksek via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 11 16:00:09 PST 2019
gokturk added a comment.
Let me try my best to explain what's happening. The goal of this check is to determine whether passing `-latomic` is required.
Let's start with the code used by `check_working_cxx_atomics64` (https://github.com/llvm/llvm-project/blob/master/llvm/cmake/modules/CheckAtomic.cmake)
#include <atomic>
#include <cstdint>
std::atomic<uint64_t> x (0);
int main() {
uint64_t i = x.load(std::memory_order_relaxed);
return 0;
}
If we compile this with `g++ -o atomic64 -march=rv64imafd atomic64.cpp`, it produces the following assembly output:
00000000000006bc <main>:
6bc: fe010113 addi sp,sp,-32
6c0: 00113c23 sd ra,24(sp)
6c4: 00813823 sd s0,16(sp)
6c8: 02010413 addi s0,sp,32
6cc: fe042023 sw zero,-32(s0)
6d0: fe042703 lw a4,-32(s0)
6d4: 000107b7 lui a5,0x10
6d8: fff78593 addi a1,a5,-1 # ffff <__global_pointer$+0xd7ff>
6dc: 00070513 mv a0,a4
6e0: 040000ef jal ra,720 <_ZStanSt12memory_orderSt23__memory_order_modifier>
6e4: 00050793 mv a5,a0
6e8: fef42223 sw a5,-28(s0)
6ec: 00002797 auipc a5,0x2
6f0: 97478793 addi a5,a5,-1676 # 2060 <x>
6f4: 0ff0000f fence
6f8: 0007b783 ld a5,0(a5)
6fc: 0ff0000f fence
700: 00000013 nop
704: fef43423 sd a5,-24(s0)
708: 00000793 li a5,0
70c: 00078513 mv a0,a5
710: 01813083 ld ra,24(sp)
714: 01013403 ld s0,16(sp)
718: 02010113 addi sp,sp,32
71c: 00008067 ret
0000000000000720 <_ZStanSt12memory_orderSt23__memory_order_modifier>:
720: fe010113 addi sp,sp,-32
724: 00813c23 sd s0,24(sp)
728: 02010413 addi s0,sp,32
72c: 00050793 mv a5,a0
730: 00058713 mv a4,a1
734: fef42623 sw a5,-20(s0)
738: 00070793 mv a5,a4
73c: fef42423 sw a5,-24(s0)
740: fec42703 lw a4,-20(s0)
744: fe842783 lw a5,-24(s0)
748: 00f777b3 and a5,a4,a5
74c: 0007879b sext.w a5,a5
750: 0007879b sext.w a5,a5
754: 00078513 mv a0,a5
758: 01813403 ld s0,24(sp)
75c: 02010113 addi sp,sp,32
760: 00008067 ret
Ignoring the fine details of the RISC-V assembly, if we just focus on `jal` instructions that are used for function calls, we see that no actual calls to libatomic are being made. This may be a separate bug of a false-positive `check_working_cxx_atomics64` test that is not addressed by this patch.
If we tweak the example a little bit to introduce a post-increment for `x`:
@@ -2,6 +2,7 @@
#include <cstdint>
std::atomic<uint64_t> x (0);
int main() {
+ x++;
uint64_t i = x.load(std::memory_order_relaxed);
return 0;
}
the post-increment generates these extra instructions in `main()`:
li a1,0
auipc a0,0x2
addi a0,a0,-1648 # 2060 <x>
jal ra,774 <_ZNSt13__atomic_baseImEppEi>
where `_ZNSt13__atomic_baseImEppEi` is:
0000000000000774 <_ZNSt13__atomic_baseImEppEi>:
774: fc010113 addi sp,sp,-64
778: 02813c23 sd s0,56(sp)
77c: 04010413 addi s0,sp,64
780: fca43423 sd a0,-56(s0)
784: 00058793 mv a5,a1
788: fcf42223 sw a5,-60(s0)
78c: fc843783 ld a5,-56(s0)
790: fef43023 sd a5,-32(s0)
794: 00100793 li a5,1
798: fef43423 sd a5,-24(s0)
79c: 00500793 li a5,5
7a0: fcf42e23 sw a5,-36(s0)
7a4: fe043783 ld a5,-32(s0)
7a8: fe843703 ld a4,-24(s0)
7ac: 0f50000f fence iorw,ow
7b0: 04e7b6af amoadd.d.aq a3,a4,(a5)
7b4: 00000013 nop
7b8: 00068793 mv a5,a3
7bc: 00078513 mv a0,a5
7c0: 03813403 ld s0,56(sp)
7c4: 04010113 addi sp,sp,64
7c8: 00008067 ret
Again ignoring the fine details of RISC-V assembly, we see that the post-increment is provided primarily by the instruction at address `0x7b0`, which is `amoadd.d` and stands for **`a`**tomic **`m`**emory **`o`**peration for a **`d`**ouble word. As a result, no calls to libatomic are generated.
If we further modify our example to use `uint8_t` instead of `uint64_t`:
@@ -1,8 +1,8 @@
#include <atomic>
#include <cstdint>
-std::atomic<uint64_t> x (0);
+std::atomic<uint8_t> x (0);
int main() {
x++;
- uint64_t i = x.load(std::memory_order_relaxed);
+ uint8_t i = x.load(std::memory_order_relaxed);
return 0;
}
and try to compile with `g++ -o atomic64 -march=rv64imafd atomic64.cpp`, it fails with the following:
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.2.0/../../../../riscv64-unknown-linux-gnu/bin/ld: /tmp/ccNet28n.o: in function `.L0 ':
atomic64.cpp:(.text._ZNSt13__atomic_baseIhEppEi[_ZNSt13__atomic_baseIhEppEi]+0x4c): undefined reference to `__atomic_fetch_add_1'
collect2: error: ld returned 1 exit status
The reason why it failed is because there are no RISC-V assembly instructions that operate on 8-bit data. So the compiler is forced to generate a call to libatomic.
Appending `-latomic` fixes the problem. A look at the assembly output roughly shows:
- 0f50000f fence iorw,ow
- 04e7b6af amoadd.d.aq a3,a4,(a5)
+ 00068613 mv a2,a3
+ 00070593 mv a1,a4
+ 00078513 mv a0,a5
+ e0dff0ef jal ra,670 <__atomic_fetch_add_1 at plt>
which clearly identifies the call to libatomic.
So how is this relevant to clang-tools-extra? ClangdLSPServer <https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/ClangdLSPServer.cpp#L309> defines `std::atomic<bool> Replied = {false};`. As a result, Replied.exchange(true) <https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/ClangdLSPServer.cpp#L350> will produce a call to `__atomic_exchange_1`. The build will fail because `check_working_cxx_atomics64` makes it seem as though passing `-latomic` is not needed where in reality it is.
I hope this clears things up a bit. I'm open to any suggestions on improving the commit message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69869/new/
https://reviews.llvm.org/D69869
More information about the cfe-commits
mailing list