Skip to content
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

Add riscv64 support #14

Merged
merged 2 commits into from Sep 17, 2021
Merged

Add riscv64 support #14

merged 2 commits into from Sep 17, 2021

Conversation

felixonmars
Copy link

Builds fine and all tests are green.

Signed-off-by: Felix Yan <felixonmars@archlinux.org>
@@ -309,6 +309,9 @@ fn main() {
} else if !target.contains("windows") {
println!("cargo:rustc-link-lib=pthread");
}
if target.contains("riscv") {
println!("cargo:rustc-link-lib=atomic");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's it for?

@MaskRay
Copy link

MaskRay commented Sep 10, 2021

cargo:rustc-link-lib=atomic is supposed to link against a library providing atomic library calls.

g in -march=rv64g expands to mafd (extensions), where a refers to atomic.

% cat a.c
#include <stdatomic.h>
_Atomic(int) a;
int get_a() { return a; }
% clang -target riscv64 -march=rv64g -O2 -S a.c
# no __atomic_ library calls

% cat b.c
#include <stdatomic.h>
_Atomic(__int128) a;
__int128 get_a() { return a; }
% clang -target riscv64 -march=rv64g -O2 -S b.c
# There is a __atomic_load call because 16-byte atomic does not have hardware support

Any idea why you need cargo:rustc-link-lib=atomic?

@felixonmars
Copy link
Author

felixonmars commented Sep 10, 2021

I got a lot of linking errors like this, without -latomic:

  = note: "cc" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.111rlyke22vc63sd.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.15ivn7s36o09rvt2.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.1k29vcrru2chf428.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.1vt200yvlkne78yv.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.21jd18jwsnyzxdpz.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.2k6l2lrn5q798yno.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.4fhntwzipneym4kd.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.4pmig1fre94lrhlz.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.57c323f5ptszf3u8.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.c9kdljvks2hlpcz.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.ghqx7c8kvov1rwv.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.inncnhr1n0rjhwy.rcgu.o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788.2sydjvb6522oeg9a.rcgu.o" "-Wl,--as-needed" "-L" "/jemallocator/target/debug/deps" "-L" "/jemallocator/target/debug/build/tikv-jemalloc-sys-69587b65cd0425c4/out/build/lib" "-L" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libtest-6f33e459c5d02ac8.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libterm-6b8200acce97bae9.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libgetopts-66e5594d9c77eeb0.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libunicode_width-eec556121125256d.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/librustc_std_workspace_std-9e64a5eb79ab4074.rlib" "/jemallocator/target/debug/deps/libtikv_jemallocator-f40d662bc8bdf70b.rlib" "/jemallocator/target/debug/deps/libtikv_jemalloc_sys-cc8dd6b7f247e9d5.rlib" "/jemallocator/target/debug/deps/liblibc-6b8633b759252212.rlib" "-Wl,--start-group" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libstd-e41e1e4d46d05d4c.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libpanic_unwind-c9ef714e5bd27bd0.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libminiz_oxide-38add971c098ca59.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libadler-ab9df3ad4577f132.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libobject-2fc3dd8617fd7a66.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libaddr2line-847160811b47488b.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libgimli-82c36476cbd74157.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libstd_detect-c1a29212f221c51b.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/librustc_demangle-ba33baaed4ea6a86.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libhashbrown-3e585d650bc65261.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/librustc_std_workspace_alloc-a67cc40249952a04.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libunwind-c52b0f39bb8e8e57.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libcfg_if-d300a7fabf6e787b.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/liblibc-4e64ef80dd530043.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/liballoc-95ec7414d74e2f9f.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/librustc_std_workspace_core-7b8598f834575129.rlib" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libcore-3533cfe92ddff54f.rlib" "-Wl,--end-group" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib/libcompiler_builtins-4be9231da2139b76.rlib" "-Wl,-Bdynamic" "-lpthread" "-lc" "-lm" "-lrt" "-lpthread" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-L" "/usr/lib/rustlib/riscv64gc-unknown-linux-gnu/lib" "-o" "/jemallocator/target/debug/deps/shrink_in_place-dbab918969ed6788" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-nodefaultlibs"
  = note: /usr/bin/ld: /jemallocator/target/debug/deps/libtikv_jemalloc_sys-cc8dd6b7f247e9d5.rlib(extent_dss.pic.o): in function `atomic_enum_to_builtin':
          /jemallocator/target/debug/build/tikv-jemalloc-sys-69587b65cd0425c4/out/build/include/jemalloc/internal/atomic_gcc_atomic.h:31: undefined reference to `__atomic_compare_exchange_1'
          /usr/bin/ld: /jemallocator/target/debug/deps/libtikv_jemalloc_sys-cc8dd6b7f247e9d5.rlib(tsd.pic.o): in function `atomic_enum_to_builtin':
          /jemallocator/target/debug/build/tikv-jemalloc-sys-69587b65cd0425c4/out/build/include/jemalloc/internal/atomic_gcc_atomic.h:31: undefined reference to `__atomic_exchange_1'
          collect2: error: ld returned 1 exit status

@MaskRay
Copy link

MaskRay commented Sep 10, 2021

OK, I believe this is a GCC deficiency riscv-collab/riscv-gcc#12

#include <stdatomic.h>
char meow(char *ptr, char val) {
  return __atomic_exchange_n(ptr, val, memory_order_seq_cst);
}

GCC may generate a __atomic_exchange_1 library call.

% riscv64-linux-gnu-gcc -march=rv64gc -O2 -S a.c -o - | grep __atomic
tail    __atomic_exchange_1@plt

Clang doesn't do this.

Mix-and-match for a given object size/alignment is not good but it should be fine. libatomic's implementation doesn't use mutex array.
compiler-rt's atomic __atomic_exchange_1 and __atomic_load_1and etc use mutex array and mis-and-match will be incorrect.

@BusyJay
Copy link
Member

BusyJay commented Sep 14, 2021

Thank you @MaskRay @felixonmars for the analyze. @felixonmars can you also add the issue link in the code as a comment to explain why atomic is linked against explicitly?

@felixonmars
Copy link
Author

Sure. Hopefully I did it right :)

@BusyJay
Copy link
Member

BusyJay commented Sep 15, 2021

You need to sign off all your commits, otherwise we can't merge your pr.

Signed-off-by: Felix Yan <felixonmars@archlinux.org>
@felixonmars
Copy link
Author

You need to sign off all your commits, otherwise we can't merge your pr.

Oops. Fixed now.

@BusyJay BusyJay merged commit 52de425 into tikv:master Sep 17, 2021
@Icenowy
Copy link

Icenowy commented Oct 1, 2021

@MaskRay mix-and-match is not okay according to https://gcc.gnu.org/wiki/Atomic/GCCMM/LIbrary .

The compiler must ensure that for any given object, it either always inlines lock free routines, or calls the external routines. For any given object, these cannot be intermixed. Ie, you can't perform a lock-free store on an object while attempting to perform a load with the external routines. All operations on the object must be one or the other. 

In this case, the behavior difference of GCC and Clang should be considered as a bug, and as (from my view) LLVM respects GCC for libatomic, Clang should be fixed to invoke atomic*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants