Wasmtime GitHub notifications bot (Apr 15 2020 at 10:51): Wasmtime GitHub notifications bot (Apr 15 2020 at 10:51):github-actions[bot] commented on Issue #1510:
Subscribe to Label Action
cc @bnjbvr
<details>
This issue or pull request has been labeled: "cranelift"Thus the following users have been cc'd because of the following labels:
- bnjbvr: cranelift
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Apr 15 2020 at 11:14):teapotd commented on Issue #1510:
Ugh, looks like it uncovered another issue - on i686 return pointer parameter will be on stack, but we're assuming it will always be in a register:
Wasmtime GitHub notifications bot (Apr 15 2020 at 12:55):teapotd commented on Issue #1510:
There was one test that didn't pass (
cranelift/filetests/filetests/legalizer/popcnt-i128.clif) and it looks like its target should bex86_64, noti686(see commit). I changed it to x86_64.It looks like function signature legalization for i686 is incomplete. In particular, it doesn't differentiate between registers designated for return values on i686 and x64. I think it should be discussed in separate issue.
Wasmtime GitHub notifications bot (Apr 15 2020 at 17:07):bjorn3 commented on Issue #1510:
For SystemV I believe
i128should be passed and returned in two registers, like it currently does, instead of throughsret.
Wasmtime GitHub notifications bot (Apr 16 2020 at 16:52):teapotd commented on Issue #1510:
@bjorn3
Okay,
i128arguments are definitely handled incorrectly and I'm not sure anymore howi128return values should be handled. The problem is that Microsoft ABI doesn't support 128-bit integer type. It defines__m128vector type, but that's different thing.A scalar return value that can fit into 64 bits is returned through RAX; this includes __m64 types. Non-scalar types including floats, doubles, and vector types such as __m128, __m128i, __m128d are returned in XMM0. [...] User-defined types can be returned by value from global functions and static member functions. To return a user-defined type by value in RAX, it must have a length of 1, 2, 4, 8, 16, 32, or 64 bits. [...] Otherwise, the caller assumes the responsibility of allocating memory and passing a pointer for the return value as the first argument. Subsequent arguments are then shifted one argument to the right. The same pointer must be returned by the callee in RAX.
So if
i128is different from__m128it should be treated like user-defined type and returned viasret. That's what rustc does forwin64calling convention and what Cranelift would do with this change. On the other hand, gcc and clang treatint128like__m128.<details><summary>msvc</summary>
#include <xmmintrin.h> __m128 test_m128(__m128 x) { return x; }x$ = 8 __m128 test_m128(__m128) PROC ; test_m128, COMDAT movups xmm0, XMMWORD PTR [rcx] ret 0 __m128 test_m128(__m128) ENDP ; test_m128</details>
<details><summary>gcc</summary>
__attribute__((ms_abi)) __int128 test_i128(__int128 x) { return x; }test_i128(__int128): movdqa xmm0, XMMWORD PTR [rcx] ret</details>
<details><summary>clang</summary>
__attribute__((ms_abi)) __int128 test_i128(__int128 x) { return x; }test_i128(__int128): # @test_i128(__int128) movaps xmm0, xmmword ptr [rcx] ret</details>
<details><summary>rustc</summary>
pub extern "win64" fn test_i128(x: i128) -> i128 { x }example::test_i128: mov rax, rcx movups xmm0, xmmword ptr [rdx] movups xmmword ptr [rcx], xmm0 ret</details>
Regarding the
i128arguments:Any argument that doesn't fit in 8 bytes, or isn't 1, 2, 4, or 8 bytes, must be passed by reference. A single argument is never spread across multiple registers. [...] 16-byte arguments are passed by reference.
From what I see, Cranelift doesn't support specifying that argument should be passed by reference.
Microsoft spec: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention
Compilers comparison: https://godbolt.org/z/bS9-FG
Wasmtime GitHub notifications bot (Apr 16 2020 at 17:14):bjorn3 commented on Issue #1510:
I would prefer matching the rustc abi for all calling conventions. This means splitting into two registers for SystemV and passing by-ref for WindowsFastCall. Matching the rustc abi is important for cg_clif to eventually be able to call between it and cg_llvm (the default backend for rustc)
Wasmtime GitHub notifications bot (Apr 29 2020 at 16:02):teapotd commented on Issue #1510:
In the current state, return values should work properly for win64 fastcall. I think fixing problems with argument passing on win64 deserves a separate PR, as it requires implementing passing-by-ref.
Wasmtime GitHub notifications bot (May 25 2020 at 13:07):bjorn3 commented on Issue #1510:
Can somebody review this PR? This is necessary for Windows support in cg_clif.
Wasmtime GitHub notifications bot (May 25 2020 at 14:50):bjorn3 commented on Issue #1510:
@teapotd I just tried this and the following resulted in a panic:
function u0:29(i128) -> i128 windows_fastcall { sig0 = (i128) -> i128 windows_fastcall fn0 = u0:35 sig0 block0(v0: i128): trap user0 block1: v6 = call fn0(v0) trap user0 }thread 'main' panicked at 'Signature still wrong: v11 = call fn0(v0, v10), sig0(i64 [%18], i64 [%24], i64 sret [%17]) -> i64 sret [%16] windows_fastcall', cranelift/codegen/src/legalizer/boundary.rs:773:5 stack backtrace: [...] 12: std::panicking::begin_panic_fmt at src/libstd/panicking.rs:332 13: cranelift_codegen::legalizer::boundary::handle_call_abi at cranelift/codegen/src/legalizer/boundary.rs:773 14: cranelift_codegen::legalizer::legalize_inst at cranelift/codegen/src/legalizer/mod.rs:58 15: cranelift_codegen::legalizer::legalize_function at cranelift/codegen/src/legalizer/mod.rs:170 16: cranelift_codegen::context::Context::legalize at cranelift/codegen/src/context.rs:319 17: cranelift_codegen::context::Context::compile at cranelift/codegen/src/context.rs:167 18: cranelift_codegen::context::Context::compile_and_emit at cranelift/codegen/src/context.rs:131 [...]
Wasmtime GitHub notifications bot (May 25 2020 at 15:47):teapotd commented on Issue #1510:
It looks like arguments to sret calls are not legalized at all (see
legalize_sret_callandhandle_call_abi). For example this gives the same panic on system_v:function u0:29(i128) { sig0 = (i128) -> i64, i64, i64, i64 system_v fn0 = u0:35 sig0 block0(v0: i128): v1, v2, v3, v4 = call fn0(v0) trap user0 }I'll look into this.
Wasmtime GitHub notifications bot (May 25 2020 at 19:29):teapotd commented on Issue #1510:
@bjorn3 It should be okay now. I think you should try #1670 instead, as it contains this changes and fixes incoming arguments as well.
Wasmtime GitHub notifications bot (May 25 2020 at 19:32):bjorn3 commented on Issue #1510:
I was trying to get it compiling first. With the latest changes and a few changes at my side to shim some inline asm with aborts it now compiles libstd fine. Linking fails for cross-compiling from linux using mingw:
= note: x86_64-w64-mingw32-gcc: error: rsbegin.o: No such file or directory x86_64-w64-mingw32-gcc: error: rsend.o: No such file or directoryThis is probably a problem with my setup though.
Wasmtime GitHub notifications bot (May 25 2020 at 20:10):bjorn3 commented on Issue #1510:
I had to copy a few object files from the cg_llvm sysroot, disable the atomic shim in cg_clif and update
objectfor a bugfix. It now gives the following when run in wine:wine ./target/out/std_example.exe bjorn@laptopbjorn-lenovo some <unknown> text cargo:rustc-link-lib=z 9.974182 4.9245777 2.3 1.5165751 2 3 1 2.3 5.29 1.2016338 5.29 wine: Unhandled page fault on read access to 0x00000000 at address 0x8a8a39 (thread 002a), starting debugger...will try the other PR tomorrow.
Wasmtime GitHub notifications bot (May 26 2020 at 09:08):bjorn3 edited a comment on Issue #1510:
I had to copy a few object files from the cg_llvm sysroot, disable the atomic shim in cg_clif and update
objectfor a bugfix. It now gives the following when run in wine:$ wine ./target/out/std_example.exe some <unknown> text cargo:rustc-link-lib=z 9.974182 4.9245777 2.3 1.5165751 2 3 1 2.3 5.29 1.2016338 5.29 wine: Unhandled page fault on read access to 0x00000000 at address 0x8a8a39 (thread 002a), starting debugger...will try the other PR tomorrow.
Wasmtime GitHub notifications bot (May 26 2020 at 10:07):bjorn3 commented on Issue #1510:
The problem was in a compiler-builtins hack to match the abi LLVM expects. It uses
#[repr(simd)]which is not implemented in cg_clif.simple-raytracer works fine now in wine.
Wasmtime GitHub notifications bot (May 26 2020 at 10:21):bjorn3 edited a comment on Issue #1510:
The problem was in a compiler-builtins hack to match the abi LLVM expects. It uses
#[repr(simd)]which is not implemented in cg_clif.simple-raytracer now works fine in wine.
Wasmtime GitHub notifications bot (May 28 2020 at 23:05):teapotd commented on Issue #1510:
@fitzgen Could you take a look at this and related #1670?
Last updated: Oct 23 2024 at 20:03 UTC