Stream: git-wasmtime

Topic: wasmtime / issue #6500 Cranelift: Get non-tail calls work...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2023 at 23:08):

fitzgen commented on issue #6500:

Looks reasonable. Could we add a comment somewhere noting that for now, the Tail convention is treated the same as SystemV, but is expected to change with subsequent patches? With that noted somewhere, LGTM.

It is already documented to have unstable ABI, so I think we should be fine on this front?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2023 at 00:35):

cfallin commented on issue #6500:

Looks reasonable. Could we add a comment somewhere noting that for now, the Tail convention is treated the same as SystemV, but is expected to change with subsequent patches? With that noted somewhere, LGTM.

It is already documented to have unstable ABI, so I think we should be fine on this front?

Sure, that's the public-facing (lack of) guarantee, but it'd be nice to have a note on the current implementation somewhere (and update it when it changes), IMHO.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2023 at 21:40):

fitzgen commented on issue #6500:

So I added another test for enough arguments that some go on the stack, and found a bug in this PR (I had the appropriate changes in another wip branch, but didn't have them here yet) where the call site was still trying to pop stack arguments, even though the callee already did that.

But then there is s390x. I don't really know s390x's calling convention, but even the simpler test that was already in this PR is failing on s390x with wrong values being returned. The new test is causing segfaults. I need to dig in more, but there is a lot for me to learn here first.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 06 2023 at 20:49):

fitzgen commented on issue #6500:

I'm having a hard time figuring out the details of s390x here, so I am going to disable support for the tail calling convention on s390x (with a loud assertion) to land this PR. We can get s390x working in follow ups.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 08 2023 at 15:42):

fitzgen commented on issue #6500:

There seem to be some wasi-http-related crashes on windows MSVC in CI: https://github.com/bytecodealliance/wasmtime/actions/runs/5193242010/jobs/9363525711?pr=6500

Has anyone seen these before? I highly doubt that this PR is the root cause, as it is changing a calling convention that isn't used by Wasmtime currently.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 08 2023 at 15:43):

fitzgen edited a comment on issue #6500:

Edit: nevermind, I completely misread some things.


Last updated: Nov 22 2024 at 16:03 UTC