Incorrect Calling Conventions on Windows

[CryZe]

Unfortunately #183 isn't actually quite resolved yet as the same problem exists for the wasm code calling into the host with an f64 argument. I believe we are dealing with S2 then or something (because of the additional context argument)? I haven't yet investigated this, so I don't quite know enough details yet.

[CryZe]

It seems like the problem there is that ctx is actually passed after the f64 or something. f64 is in the XMM0 register even though the ABI requires it to be in the XMM1 register if it's the second argument, which is also not the case for Linux, so this would explain this being a Windows only problem again. I think I saw the ctx argument being moved to the first parameter in some PR. Maybe it didn't do that all the way?

Here you can see XMM1 being used:
https://rust.godbolt.org/z/v4BTVw

Here's the jitted code filling the value into XMM0:

Alternatively, cranelift may not know about the Windows calling convention and thus just fill the value into XMM0 like it would on Linux (either it isn't being told about it, or it doesn't support it yet).

[CryZe]

Defining the wrap function to always be of "sysv64" calling convention fixes the bug, but that seems more like a workaround rather than a proper fix, as cranelift should probably just emit the correct calling convention in the first place. I don't know where this is done though. (Update: This fixes some things but breaks others, I'm thinking cranelift actually is going for the windows calling convention but the ctx parameter isn't implemented correctly)

I'd encourage some maintainer to verify if all the calling conventions between the jit'ed code and rust are actually correct (all the trampolines, libcalls, ...).

[lachlansneff]

@CryZe Thanks for the deep dive! I "own" most of the runtime code, so I'll take a look at this on Monday. It's probably the calling convention getting set up wrong somewhere.

[lachlansneff]

@CryZe Sorry this took so long, but I've just had time to investigate and indeed yes, cranelift implements the x64 calling convention incorrectly. I honestly don't blame them, it seems to waste registers.

Here's the spec: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2017#example-of-argument-passing-3---mixed-ints-and-floats

[syrusakbary]

We created an issue in Cranelift: bytecodealliance/cranelift#691

[syrusakbary]

This issue was fixed in: bytecodealliance/cranelift#701 (related issue in cranelift: bytecodealliance/cranelift#691)

Once we update the Cranelift version the issue should be fixed in our runtime.

Keeping the issue open until then!

[CryZe]

The implementation was unfortunately incorrect. So they still need to fix that. (Oh wait, they fixed that too).

[hrydgard]

I had similar problems with floating point arguments on Windows, but indeed appears to work correctly on master with the the updated cranelift.

Are you guys planning to tag a new release in the near future? Would be convenient :)

[CryZe]

This seems to be resolved :)