aarch64: Implement TLS ELF GD Relocations#3026
Conversation
| &mut Inst::EmitIsland { .. } => {} | ||
|
|
||
| &mut Inst::ElfTlsGetAddr { .. } => { | ||
| // TODO: This is probably wrong |
There was a problem hiding this comment.
This is correct. All registers are fixed, so marking them as clobbered in aarch64_get_regs is enough. There are no registers that need to be remapped.
wasmtime/cranelift/src/compile.rs Line 71 in 03077e0 |
|
Oh, that's really nice, I didn't see that flag, here's what that outputs: So, it is doing the relocations, but i don't think those are the correct offsets to be emitting? |
|
Looks like it is off by 4 bytes if these offsets are in decimal. |
|
I think you need to move the relocs before the |
|
Yeah, that seems to be the right thing to do, we also put relocations before the call instruction and I didn't notice it before: wasmtime/cranelift/codegen/src/isa/aarch64/inst/emit.rs Lines 2295 to 2296 in 8172620 Unfortunately we get the same result, but I did another test, by compiling the following file with gcc and dumping relocations: We get: Which is not the relocation we are emitting. I'm going to fix that now |
|
Before linking there is: which seems correct. However after linking it turns into: which has an undefined instruction. Edit: missed your above comment. |
6065928 to
bf89839
Compare
|
I've updated it now, we should be emitting exactly what gcc is doing now. By the way, can you tell me the steps to reproduce the test you are doing? |
|
I am trying to run the test suite of cg_clif using diff --git a/Cargo.toml b/Cargo.toml
index ef68d7ee..f2bf9536 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -17,7 +17,7 @@ cranelift-jit = { git = "https://github.com/bytecodealliance/wasmtime.git", bran
cranelift-object = { git = "https://github.com/bytecodealliance/wasmtime.git", branch = "main" }
target-lexicon = "0.12.0"
gimli = { version = "0.24.0", default-features = false, features = ["write"]}
-object = { version = "0.25.0", default-features = false, features = ["std", "read_core", "write", "archive", "coff", "elf", "macho", "pe"] }
+object = { version = "0.25.3", default-features = false, features = ["std", "read_core", "write", "archive", "coff", "elf", "macho", "pe"] }
ar = { git = "https://github.com/bjorn3/rust-ar.git", branch = "do_not_remove_cg_clif_ranlib" }
indexmap = "1.0.2"
@@ -25,13 +25,13 @@ libloading = { version = "0.6.0", optional = true }
smallvec = "1.6.1"
# Uncomment to use local checkout of cranelift
-#[patch."https://github.com/bytecodealliance/wasmtime.git"]
-#cranelift-codegen = { path = "../wasmtime/cranelift/codegen" }
-#cranelift-frontend = { path = "../wasmtime/cranelift/frontend" }
-#cranelift-module = { path = "../wasmtime/cranelift/module" }
-#cranelift-native = { path = "../wasmtime/cranelift/native" }
-#cranelift-jit = { path = "../wasmtime/cranelift/jit" }
-#cranelift-object = { path = "../wasmtime/cranelift/object" }
+[patch."https://github.com/bytecodealliance/wasmtime.git"]
+cranelift-codegen = { path = "../wasmtime/cranelift/codegen" }
+cranelift-frontend = { path = "../wasmtime/cranelift/frontend" }
+cranelift-module = { path = "../wasmtime/cranelift/module" }
+cranelift-native = { path = "../wasmtime/cranelift/native" }
+cranelift-jit = { path = "../wasmtime/cranelift/jit" }
+cranelift-object = { path = "../wasmtime/cranelift/object" }
#[patch.crates-io]
#gimli = { path = "../" }
diff --git a/example/mini_core_hello_world.rs b/example/mini_core_hello_world.rs
index 6570f2bf..3d2d0303 100644
--- a/example/mini_core_hello_world.rs
+++ b/example/mini_core_hello_world.rs
@@ -294,7 +294,7 @@ struct ExternTypeWrapper {
#[cfg(all(not(jit), target_os = "linux"))]
unsafe {
- global_asm_test();
+ //global_asm_test();
}
// Both statics have a reference that points to the same anonymous allocation.
@@ -309,14 +309,14 @@ struct ExternTypeWrapper {
}
#[cfg(all(not(jit), target_os = "linux"))]
-global_asm! {
+/*global_asm! {
"
.global global_asm_test
global_asm_test:
// comment that would normally be removed by LLVM
ret
"
-}
+}*/
#[repr(C)]
enum c_void {
diff --git a/src/lib.rs b/src/lib.rs
index aed25a48..956113b2 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -289,7 +289,7 @@ fn build_isa(sess: &Session, backend_config: &BackendConfig) -> Box<dyn isa::Tar
cranelift_codegen::isa::lookup_variant(target_triple, variant).unwrap();
// Don't use "haswell" as the default, as it implies `has_lzcnt`.
// macOS CI is still at Ivy Bridge EP, so `lzcnt` is interpreted as `bsr`.
- builder.enable("nehalem").unwrap();
+ //builder.enable("nehalem").unwrap();
builder
}
}; |
|
mini_core_hello_world runs fine now! |
Implement the `TlsValue` opcode in the aarch64 backend for ELF_GD. This is a little bit unusual as the default TLS mechanism for aarch64 is TLS Descriptors in other compilers. However currently we only recognize elf_gd so lets start with that as a TLS implementation.
bf89839 to
b8ad99e
Compare
|
libstd compiles fine too with a small patch to the build system to fix cross-compiling. (I recently rewrote it from shell scripts to rust) std_example doesn't compile yet. I think due to a |
That's awesome! Although it would be nice if we had a more definitive test that would catch issues like this (Wrong relocations). @bjorn3 I'm having some issues running cg_clif's test suite, is there anywhere I can ask for a bit more guidance, so we don't pollute this PR?
I guess that's next on the i128 impl list |
|
When commenting out all i128 tests in std_example, it runs fine too.
Maybe zulip? You can PM me on either the rust-lang or bytecodealliance zulip. |
Yeah, I agree this would be nice; we don't really have a good answer for this right now. Probably the best answer would be an end-to-end test of some sort that uses the relocation and executes on the target platform. In the absence of that, we could at least print relocations in the VCode output and check them (as a golden output) in compile filetests. I'd be happy to review something for that if you would like. |
cfallin
left a comment
There was a problem hiding this comment.
Looks great, thanks! I'm really happy that cg_clif on aarch64 is so close to working.
Will merge once the whitespace-at-end-of-line nit commented on by bjorn3 is fixed.
|
Oops, it was fixed, but i forgot to mark it as resolved. Thanks
Hmm, its not something i want to pick up right now, but let me open an issue with this, and maybe someone does |
Hey,
I've been working on this, but I'm a bit stuck and need some help.
This PR Implements TLS ELF GD relocations for aarch64. ELF GD Relocations are a bit unusual in aarch64 since the default is TLS descriptors (at least in gcc/clang).
Right now I don't think the relocations are being performed correctly.
The only way I have to test this is to run the following command:
clif-util compile -D --set tls_model=elf_gd --target aarch64 ./filetests/filetests/isa/aarch64/tls-elf-gd.clifWhich emits the following code:
I suspect that this means that the first two relocations are silently not being done?
The other issue that I have, is that I haven't found a way to create a runtest that tests this.